Remove need to touch lock files before using.

- Locks will now create enclosing directories and touch the lock file
  automatically.

6 files changed, 68 insertions, 38 deletions

diff --git a/lib/spack/llnl/util/lock.py b/lib/spack/llnl/util/lock.py
index ce31a59d62..3ebbf25eb8 100644
--- a/lib/spack/llnl/util/lock.py
+++ b/lib/spack/llnl/util/lock.py
@@ -52,12 +52,16 @@ class Lock(object):
 
     """
 
-    def __init__(self, file_path):
-        self._file_path = file_path
+    def __init__(self, path):
+        self.path = path
         self._file = None
         self._reads = 0
         self._writes = 0
 
+        # PID and host of lock holder
+        self.pid = self.old_pid = None
+        self.host = self.old_host = None
+
     def _lock(self, op, timeout):
         """This takes a lock using POSIX locks (``fnctl.lockf``).
 
@@ -83,15 +87,25 @@ class Lock(object):
                 # 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)
+                    self._ensure_parent_directory()
+
+                    os_mode, fd_mode = os.O_RDONLY, 'r'
+                    if op == fcntl.LOCK_EX or not os.path.exists(self.path):
+                        os_mode, fd_mode = (os.O_RDWR | os.O_CREAT), 'r+'
 
+                    fd = os.open(self.path, os_mode)
+                    self._file = os.fdopen(fd, fd_mode)
+
+                # Try to get the lock (will raise if not available.)
                 fcntl.lockf(self._file, op | fcntl.LOCK_NB)
+
+                # All locks read the owner PID and host
+                self._read_lock_data()
+
+                # Exclusive locks write their PID/host
                 if op == fcntl.LOCK_EX:
-                    self._file.write(
-                        "pid=%s,host=%s" % (os.getpid(), socket.getfqdn()))
-                    self._file.truncate()
-                    self._file.flush()
+                    self._write_lock_data()
+
                 return
 
             except IOError as error:
@@ -103,6 +117,40 @@ class Lock(object):
 
         raise LockError("Timed out waiting for lock.")
 
+    def _ensure_parent_directory(self):
+        parent = os.path.dirname(self.path)
+        try:
+            os.makedirs(parent)
+            return True
+        except OSError as e:
+            # makedirs can fail when diretory already exists.
+            if not (e.errno == errno.EEXIST and os.path.isdir(parent) or
+                    e.errno == errno.EISDIR):
+                raise
+
+    def _read_lock_data(self):
+        """Read PID and host data out of the file if it is there."""
+        line = self._file.read()
+        if line:
+            pid, host = line.strip().split(',')
+            _, _, self.pid = pid.rpartition('=')
+            _, _, self.host = host.rpartition('=')
+
+    def _write_lock_data(self):
+        """Write PID and host data to the file, recording old values."""
+        self.old_pid = self.pid
+        self.old_host = self.host
+
+        self.pid = os.getpid()
+        self.host = socket.getfqdn()
+
+        # write pid, host to disk to sync over FS
+        self._file.seek(0)
+        self._file.write("pid=%s,host=%s" % (self.pid, self.host))
+        self._file.truncate()
+        self._file.flush()
+        os.fsync(self._file.fileno())
+
     def _unlock(self):
         """Releases a lock using POSIX locks (``fcntl.lockf``)
 
@@ -126,7 +174,7 @@ class Lock(object):
 
         """
         if self._reads == 0 and self._writes == 0:
-            tty.debug('READ LOCK : {0._file_path} [Acquiring]'.format(self))
+            tty.debug('READ LOCK : {0.path} [Acquiring]'.format(self))
             self._lock(fcntl.LOCK_SH, timeout)   # can raise LockError.
             self._reads += 1
             return True
@@ -146,7 +194,7 @@ class Lock(object):
 
         """
         if self._writes == 0:
-            tty.debug('WRITE LOCK : {0._file_path} [Acquiring]'.format(self))
+            tty.debug('WRITE LOCK : {0.path} [Acquiring]'.format(self))
             self._lock(fcntl.LOCK_EX, timeout)   # can raise LockError.
             self._writes += 1
             return True
@@ -167,7 +215,7 @@ class Lock(object):
         assert self._reads > 0
 
         if self._reads == 1 and self._writes == 0:
-            tty.debug('READ LOCK : {0._file_path} [Released]'.format(self))
+            tty.debug('READ LOCK : {0.path} [Released]'.format(self))
             self._unlock()      # can raise LockError.
             self._reads -= 1
             return True
@@ -188,7 +236,7 @@ class Lock(object):
         assert self._writes > 0
 
         if self._writes == 1 and self._reads == 0:
-            tty.debug('WRITE LOCK : {0._file_path} [Released]'.format(self))
+            tty.debug('WRITE LOCK : {0.path} [Released]'.format(self))
             self._unlock()      # can raise LockError.
             self._writes -= 1
             return True
diff --git a/lib/spack/spack/database.py b/lib/spack/spack/database.py
index 8c29ceeb27..0af08e9449 100644
--- a/lib/spack/spack/database.py
+++ b/lib/spack/spack/database.py
@@ -160,9 +160,6 @@ class Database(object):
         if not os.path.exists(self._db_dir):
             mkdirp(self._db_dir)
 
-        if not os.path.exists(self._lock_path):
-            touch(self._lock_path)
-
         # initialize rest of state.
         self.lock = Lock(self._lock_path)
         self._data = {}
diff --git a/lib/spack/spack/file_cache.py b/lib/spack/spack/file_cache.py
index 0a66166fd8..31ae009836 100644
--- a/lib/spack/spack/file_cache.py
+++ b/lib/spack/spack/file_cache.py
@@ -77,10 +77,7 @@ class FileCache(object):
     def _get_lock(self, key):
         """Create a lock for a key, if necessary, and return a lock object."""
         if key not in self._locks:
-            lock_file = self._lock_path(key)
-            if not os.path.exists(lock_file):
-                touch(lock_file)
-            self._locks[key] = Lock(lock_file)
+            self._locks[key] = Lock(self._lock_path(key))
         return self._locks[key]
 
     def init_entry(self, key):
diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py
index 8c00ff9741..9b1c6b11a2 100644
--- a/lib/spack/spack/package.py
+++ b/lib/spack/spack/package.py
@@ -703,17 +703,8 @@ class Package(object):
         if self._prefix_lock is None:
             dirname = join_path(os.path.dirname(self.spec.prefix), '.locks')
             basename = os.path.basename(self.spec.prefix)
-            lock_file = join_path(dirname, basename)
-
-            if not os.path.exists(lock_file):
-                tty.debug('TOUCH FILE : {0}'.format(lock_file))
-                try:
-                    os.makedirs(dirname)
-                except OSError:
-                    pass
-                touch(lock_file)
-
-            self._prefix_lock = llnl.util.lock.Lock(lock_file)
+            self._prefix_lock = llnl.util.lock.Lock(
+                join_path(dirname, basename))
 
         return self._prefix_lock
 
diff --git a/lib/spack/spack/stage.py b/lib/spack/spack/stage.py
index 8fcac222a0..f282c3e1c0 100644
--- a/lib/spack/spack/stage.py
+++ b/lib/spack/spack/stage.py
@@ -150,15 +150,10 @@ class Stage(object):
         self.keep = keep
 
         # File lock for the stage directory
-        self._lock_file = None
         self._lock = None
         if lock:
-            self._lock_file = join_path(spack.stage_path, self.name + '.lock')
-            if not os.path.exists(self._lock_file):
-                directory, _ = os.path.split(self._lock_file)
-                mkdirp(directory)
-                touch(self._lock_file)
-            self._lock = llnl.util.lock.Lock(self._lock_file)
+            self._lock = llnl.util.lock.Lock(
+                join_path(spack.stage_path, self.name + '.lock'))
 
     def __enter__(self):
         """
diff --git a/lib/spack/spack/test/lock.py b/lib/spack/spack/test/lock.py
index 30b7dbce0e..a3b8a3e11a 100644
--- a/lib/spack/spack/test/lock.py
+++ b/lib/spack/spack/test/lock.py
@@ -44,7 +44,6 @@ class LockTest(unittest.TestCase):
     def setUp(self):
         self.tempdir = tempfile.mkdtemp()
         self.lock_path = join_path(self.tempdir, 'lockfile')
-        touch(self.lock_path)
 
     def tearDown(self):
         shutil.rmtree(self.tempdir, ignore_errors=True)
@@ -172,14 +171,17 @@ class LockTest(unittest.TestCase):
         lock.acquire_read()
         self.assertTrue(lock._reads == 1)
         self.assertTrue(lock._writes == 0)
+        self.assertTrue(lock._file.mode == 'r')
 
         lock.acquire_write()
         self.assertTrue(lock._reads == 1)
         self.assertTrue(lock._writes == 1)
+        self.assertTrue(lock._file.mode == 'r+')
 
         lock.release_write()
         self.assertTrue(lock._reads == 1)
         self.assertTrue(lock._writes == 0)
+        self.assertTrue(lock._file.mode == 'r+')
 
         lock.release_read()
         self.assertTrue(lock._reads == 0)