From 3d8d8d3644d47c86ed8f77c488355ac1db1c8d42 Mon Sep 17 00:00:00 2001
From: Todd Gamblin <tgamblin@llnl.gov>
Date: Thu, 6 Oct 2016 00:31:19 -0700
Subject: [PATCH] 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.
---
lib/spack/llnl/util/lock.py | 28 ++++++++++++++++------------
1 file 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 @@ def _lock(self, op, timeout=_default_timeout):
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)
--
GitLab