diff --git a/lib/spack/llnl/util/lock.py b/lib/spack/llnl/util/lock.py index ce31a59d627e139c60251ffae7172c0f499eb76e..3ebbf25eb8ed7cd77f90269e1a253c47e5b2dbc3 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 @@ def _lock(self, op, timeout): # 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 @@ def _lock(self, op, timeout): 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 @@ def acquire_read(self, timeout=_default_timeout): """ 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 @@ def acquire_write(self, timeout=_default_timeout): """ 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 @@ def release_read(self): 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 @@ def release_write(self): 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 8c29ceeb27d9d7dc15cea91c419b19cf429f44ce..0af08e9449df095eaa819ad57bd6f76d8b9f8756 100644 --- a/lib/spack/spack/database.py +++ b/lib/spack/spack/database.py @@ -160,9 +160,6 @@ def __init__(self, root, db_dir=None): 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 0a66166fd8f6ca7612a0a78305379e45a3b72a73..31ae009836191268c1192fa103f0d52f11e473bc 100644 --- a/lib/spack/spack/file_cache.py +++ b/lib/spack/spack/file_cache.py @@ -77,10 +77,7 @@ def _lock_path(self, key): 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 8c00ff9741993d0e6c26c2564131f8379648de76..9b1c6b11a229505d965720bd46cbf77a8da8f8df 100644 --- a/lib/spack/spack/package.py +++ b/lib/spack/spack/package.py @@ -703,17 +703,8 @@ def prefix_lock(self): 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 8fcac222a0f1964fa74b284ef7b0c888fe975ca7..f282c3e1c00dbc1eefb64bad5cc3970e35e102f6 100644 --- a/lib/spack/spack/stage.py +++ b/lib/spack/spack/stage.py @@ -150,15 +150,10 @@ def __init__( 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 30b7dbce0e941c17150f923352e98409902c0e5a..a3b8a3e11aad77035ef186e39b8da9ddbcf0933c 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 @@ def test_upgrade_read_to_write(self): 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)