summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTodd Gamblin <tgamblin@llnl.gov>2016-08-20 16:36:40 -0700
committerTodd Gamblin <tgamblin@llnl.gov>2016-10-11 01:55:32 -0700
commitea10e3bab04a7bc89d86cf2f56a32d08ab4e3b21 (patch)
tree75d233f81de55e1c2ef487bc4f21147669ca6f54
parent907fe912ef62501d4379635ff1dfe9cea921cec3 (diff)
downloadspack-ea10e3bab04a7bc89d86cf2f56a32d08ab4e3b21.tar.gz
spack-ea10e3bab04a7bc89d86cf2f56a32d08ab4e3b21.tar.bz2
spack-ea10e3bab04a7bc89d86cf2f56a32d08ab4e3b21.tar.xz
spack-ea10e3bab04a7bc89d86cf2f56a32d08ab4e3b21.zip
Remove need to touch lock files before using.
- Locks will now create enclosing directories and touch the lock file automatically.
-rw-r--r--lib/spack/llnl/util/lock.py72
-rw-r--r--lib/spack/spack/database.py3
-rw-r--r--lib/spack/spack/file_cache.py5
-rw-r--r--lib/spack/spack/package.py13
-rw-r--r--lib/spack/spack/stage.py9
-rw-r--r--lib/spack/spack/test/lock.py4
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)