summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTodd Gamblin <tgamblin@llnl.gov>2016-10-06 00:31:19 -0700
committerTodd Gamblin <tgamblin@llnl.gov>2016-10-11 01:55:33 -0700
commit3d8d8d3644d47c86ed8f77c488355ac1db1c8d42 (patch)
tree319a3f83cf6307235bf09ae2b5b30b719424397c
parenta024c6df954fc43f5d47e788130b9af123bd4cdf (diff)
downloadspack-3d8d8d3644d47c86ed8f77c488355ac1db1c8d42.tar.gz
spack-3d8d8d3644d47c86ed8f77c488355ac1db1c8d42.tar.bz2
spack-3d8d8d3644d47c86ed8f77c488355ac1db1c8d42.tar.xz
spack-3d8d8d3644d47c86ed8f77c488355ac1db1c8d42.zip
Fix bug with lock upgrades.
- Closing and re-opening to upgrade to write will lose all existing read locks on this process. - If we didn't allow ranges, sleeping until no reads would work. - With ranges, we may never be able to take some legal write locks without invalidating all reads. e.g., if a write lock has distinct range from all reads, it should just work, but we'd have to close the file, reopen, and re-take reads. - It's easier to just check whether the file is writable in the first place and open for writing from the start. - Lock now only opens files read-only if we *can't* write them.
-rw-r--r--lib/spack/llnl/util/lock.py28
1 files changed, 16 insertions, 12 deletions
diff --git a/lib/spack/llnl/util/lock.py b/lib/spack/llnl/util/lock.py
index 86ad9d60e1..2e44a94798 100644
--- a/lib/spack/llnl/util/lock.py
+++ b/lib/spack/llnl/util/lock.py
@@ -91,21 +91,25 @@ 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._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 we could write the file, we'd have opened it 'r+'.
+ # Raise an error when we attempt to upgrade to a write lock.
+ if op == fcntl.LOCK_EX:
+ if self._file and self._file.mode == 'r':
+ raise LockError(
+ "Can't take exclusive lock on read-only file: %s"
+ % self.path)
+
+ # Create file and parent directories if they don't exist.
if self._file is None:
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+'
+ # Prefer to open 'r+' to allow upgrading to write
+ # lock later if possible. Open read-only if we can't
+ # write the lock file at all.
+ os_mode, fd_mode = (os.O_RDWR | os.O_CREAT), 'r+'
+ if os.path.exists(self.path) and not os.access(
+ self.path, os.W_OK):
+ os_mode, fd_mode = os.O_RDONLY, 'r'
fd = os.open(self.path, os_mode)
self._file = os.fdopen(fd, fd_mode)