Read-only locks should close fd before opening for write. (#1906)

- Fixes bad file descriptor error in lock acquire, #1904
- Fix bug introduced in previous PR #1857
- Backported fix from soon-to-be merged fine-grained DB locking branch.

2 files changed, 37 insertions, 0 deletions

diff --git a/lib/spack/llnl/util/lock.py b/lib/spack/llnl/util/lock.py
index b5e3a3a8f8..f5f53101ae 100644
--- a/lib/spack/llnl/util/lock.py
+++ b/lib/spack/llnl/util/lock.py
@@ -69,6 +69,14 @@ class Lock(object):
         start_time = time.time()
         while (time.time() - start_time) < timeout:
             try:
+                # If this is already open read-only and we want to
+                # upgrade to an exclusive write lock, close first.
+                if self._fd is not None:
+                    flags = fcntl.fcntl(self._fd, fcntl.F_GETFL)
+                    if op == fcntl.LOCK_EX and flags | os.O_RDONLY:
+                        os.close(self._fd)
+                        self._fd = None
+
                 if self._fd is None:
                     mode = os.O_RDWR if op == fcntl.LOCK_EX else os.O_RDONLY
                     self._fd = os.open(self._file_path, mode)
diff --git a/lib/spack/spack/test/lock.py b/lib/spack/spack/test/lock.py
index fb96539897..32cbe13ce1 100644
--- a/lib/spack/spack/test/lock.py
+++ b/lib/spack/spack/test/lock.py
@@ -158,6 +158,35 @@ class LockTest(unittest.TestCase):
             self.timeout_write, self.timeout_write)
 
     #
+    # Test that read can be upgraded to write.
+    #
+    def test_upgrade_read_to_write(self):
+        # ensure lock file exists the first time, so we open it read-only
+        # to begin wtih.
+        touch(self.lock_path)
+
+        lock = Lock(self.lock_path)
+        self.assertTrue(lock._reads == 0)
+        self.assertTrue(lock._writes == 0)
+
+        lock.acquire_read()
+        self.assertTrue(lock._reads == 1)
+        self.assertTrue(lock._writes == 0)
+
+        lock.acquire_write()
+        self.assertTrue(lock._reads == 1)
+        self.assertTrue(lock._writes == 1)
+
+        lock.release_write()
+        self.assertTrue(lock._reads == 1)
+        self.assertTrue(lock._writes == 0)
+
+        lock.release_read()
+        self.assertTrue(lock._reads == 0)
+        self.assertTrue(lock._writes == 0)
+        self.assertTrue(lock._fd is None)
+
+    #
     # Longer test case that ensures locks are reusable. Ordering is
     # enforced by barriers throughout -- steps are shown with numbers.
     #