diff options
| -rw-r--r-- | lib/spack/llnl/util/lock.py | 147 |
1 files changed, 127 insertions, 20 deletions
diff --git a/lib/spack/llnl/util/lock.py b/lib/spack/llnl/util/lock.py index 5fd7163e2e..396f941616 100644 --- a/lib/spack/llnl/util/lock.py +++ b/lib/spack/llnl/util/lock.py @@ -26,6 +26,126 @@ lock_type = {fcntl.LOCK_SH: 'read', fcntl.LOCK_EX: 'write'} true_fn = lambda: True +class OpenFile(object): + """Record for keeping track of open lockfiles (with reference counting). + + There's really only one ``OpenFile`` per inode, per process, but we record the + filehandle here as it's the thing we end up using in python code. You can get + the file descriptor from the file handle if needed -- or we could make this track + file descriptors as well in the future. + """ + def __init__(self, fh): + self.fh = fh + self.refs = 0 + + +class OpenFileTracker(object): + """Track open lockfiles, to minimize number of open file descriptors. + + The ``fcntl`` locks that Spack uses are associated with an inode and a process. + This is convenient, because if a process exits, it releases its locks. + Unfortunately, this also means that if you close a file, *all* locks associated + with that file's inode are released, regardless of whether the process has any + other open file descriptors on it. + + Because of this, we need to track open lock files so that we only close them when + a process no longer needs them. We do this by tracking each lockfile by its + inode and process id. This has several nice properties: + + 1. Tracking by pid ensures that, if we fork, we don't inadvertently track the parent + process's lockfiles. ``fcntl`` locks are not inherited across forks, so we'll + just track new lockfiles in the child. + 2. Tracking by inode ensures that referencs are counted per inode, and that we don't + inadvertently close a file whose inode still has open locks. + 3. Tracking by both pid and inode ensures that we only open lockfiles the minimum + number of times necessary for the locks we have. + + Note: as mentioned elsewhere, these locks aren't thread safe -- they're designed to + work in Python and assume the GIL. + """ + + def __init__(self): + """Create a new ``OpenFileTracker``.""" + self._descriptors = {} + + def get_fh(self, path): + """Get a filehandle for a lockfile. + + This routine will open writable files for read/write even if you're asking + for a shared (read-only) lock. This is so that we can upgrade to an exclusive + (write) lock later if requested. + + Arguments: + path (str): path to lock file we want a filehandle for + """ + # Open writable files as 'r+' so we can upgrade to write later + os_mode, fh_mode = (os.O_RDWR | os.O_CREAT), 'r+' + + pid = os.getpid() + open_file = None # OpenFile object, if there is one + stat = None # stat result for the lockfile, if it exists + + try: + # see whether we've seen this inode/pid before + stat = os.stat(path) + key = (stat.st_ino, pid) + open_file = self._descriptors.get(key) + + except OSError as e: + if e.errno != errno.ENOENT: # only handle file not found + raise + + # path does not exist -- fail if we won't be able to create it + parent = os.path.dirname(path) or '.' + if not os.access(parent, os.W_OK): + raise CantCreateLockError(path) + + # if there was no already open file, we'll need to open one + if not open_file: + if stat and not os.access(path, os.W_OK): + # we know path exists but not if it's writable. If it's read-only, + # only open the file for reading (and fail if we're trying to get + # an exclusive (write) lock on it) + os_mode, fh_mode = os.O_RDONLY, 'r' + + fd = os.open(path, os_mode) + fh = os.fdopen(fd, fh_mode) + open_file = OpenFile(fh) + + # if we just created the file, we'll need to get its inode here + if not stat: + inode = os.fstat(fd).st_ino + key = (inode, pid) + + self._descriptors[key] = open_file + + open_file.refs += 1 + return open_file.fh + + def release_fh(self, path): + """Release a filehandle, only closing it if there are no more references.""" + try: + inode = os.stat(path).st_ino + except OSError as e: + if e.errno != errno.ENOENT: # only handle file not found + raise + inode = None # this will not be in self._descriptors + + key = (inode, os.getpid()) + open_file = self._descriptors.get(key) + assert open_file, "Attempted to close non-existing lock path: %s" % path + + open_file.refs -= 1 + if not open_file.refs: + del self._descriptors[key] + open_file.fh.close() + + +#: Open file descriptors for locks in this process. Used to prevent one process +#: from opening the sam file many times for different byte range locks +file_tracker = OpenFileTracker() + + def _attempts_str(wait_time, nattempts): # Don't print anything if we succeeded on the first try if nattempts <= 1: @@ -46,7 +166,8 @@ class Lock(object): Note that this is for managing contention over resources *between* processes and not for managing contention between threads in a process: the functions of this object are not thread-safe. A process also must not - maintain multiple locks on the same file. + maintain multiple locks on the same file (or, more specifically, on + overlapping byte ranges in the same file). """ def __init__(self, path, start=0, length=0, default_timeout=None, @@ -151,25 +272,10 @@ class Lock(object): # Create file and parent directories if they don't exist. if self._file is None: - parent = self._ensure_parent_directory() + self._ensure_parent_directory() + self._file = file_tracker.get_fh(self.path) - # Open writable files as 'r+' so we can upgrade to write later - os_mode, fd_mode = (os.O_RDWR | os.O_CREAT), 'r+' - if os.path.exists(self.path): - if not os.access(self.path, os.W_OK): - if op == fcntl.LOCK_SH: - # can still lock read-only files if we open 'r' - os_mode, fd_mode = os.O_RDONLY, 'r' - else: - raise LockROFileError(self.path) - - elif not os.access(parent, os.W_OK): - raise CantCreateLockError(self.path) - - fd = os.open(self.path, os_mode) - self._file = os.fdopen(fd, fd_mode) - - elif op == fcntl.LOCK_EX and self._file.mode == 'r': + if op == fcntl.LOCK_EX and self._file.mode == 'r': # Attempt to upgrade to write lock w/a read-only file. # If the file were writable, we'd have opened it 'r+' raise LockROFileError(self.path) @@ -282,7 +388,8 @@ class Lock(object): """ fcntl.lockf(self._file, fcntl.LOCK_UN, self._length, self._start, os.SEEK_SET) - self._file.close() + + file_tracker.release_fh(self.path) self._file = None self._reads = 0 self._writes = 0 |