Make llnl.util.lock use file objects instead of low-level OS fds.

- Make sure we write, truncate, flush when setting PID and owning host in
  the file.

2 files changed, 20 insertions, 18 deletions

diff --git a/lib/spack/llnl/util/lock.py b/lib/spack/llnl/util/lock.py
index 2cde389bd2..ce31a59d62 100644
--- a/lib/spack/llnl/util/lock.py
+++ b/lib/spack/llnl/util/lock.py
@@ -54,7 +54,7 @@ class Lock(object):
 
     def __init__(self, file_path):
         self._file_path = file_path
-        self._fd = None
+        self._file = None
         self._reads = 0
         self._writes = 0
 
@@ -75,21 +75,23 @@ class Lock(object):
             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)
-
-                fcntl.lockf(self._fd, op | fcntl.LOCK_NB)
+                if self._file is not None:
+                    if op == fcntl.LOCK_EX and self._file.mode == 'r':
+                        self._file.close()
+                        self._file = None
+
+                # Open reader locks read-only if possible.
+                # lock doesn't exist, open RW + create if it doesn't exist.
+                if self._file is None:
+                    mode = 'r+' if op == fcntl.LOCK_EX else 'r'
+                    self._file = open(self._file_path, mode)
+
+                fcntl.lockf(self._file, op | fcntl.LOCK_NB)
                 if op == fcntl.LOCK_EX:
-                    os.write(
-                        self._fd,
+                    self._file.write(
                         "pid=%s,host=%s" % (os.getpid(), socket.getfqdn()))
+                    self._file.truncate()
+                    self._file.flush()
                 return
 
             except IOError as error:
@@ -108,9 +110,9 @@ class Lock(object):
         be masquerading as write locks, but this removes either.
 
         """
-        fcntl.lockf(self._fd, fcntl.LOCK_UN)
-        os.close(self._fd)
-        self._fd = None
+        fcntl.lockf(self._file, fcntl.LOCK_UN)
+        self._file.close()
+        self._file = None
 
     def acquire_read(self, timeout=_default_timeout):
         """Acquires a recursive, shared lock for reading.
diff --git a/lib/spack/spack/test/lock.py b/lib/spack/spack/test/lock.py
index 32cbe13ce1..30b7dbce0e 100644
--- a/lib/spack/spack/test/lock.py
+++ b/lib/spack/spack/test/lock.py
@@ -184,7 +184,7 @@ class LockTest(unittest.TestCase):
         lock.release_read()
         self.assertTrue(lock._reads == 0)
         self.assertTrue(lock._writes == 0)
-        self.assertTrue(lock._fd is None)
+        self.assertTrue(lock._file is None)
 
     #
     # Longer test case that ensures locks are reusable. Ordering is