devserver: enhance static subdirectory locking
Shift to using a dedicated lock file in each static subdirectory as an
indicator for exclusive access. This frees us from the implicit
semantics by which a lock on each subdirectory can be obtained at most
once, which is necessary in cases where we revisit a preexisting
directory and stage additional artifacts in it.
- The previous locking behavior is retained by default; a client must
use an optional argument to AcquireLock() to indicate that the locked
subdirectory may preexist.
- Uses lockfile.FileLock for the actual locking. This is deemed more
straightforward and flexible than interprocess locking: we need not
map resources (directories) to locks, and our implementation is not
limited to a single multithreaded process.
- ReleaseLock() does not remove the locked subdirectory, unless
explicitly instructed to. Added code for graceful (non-destructive)
release in successful branches.
- Test coverage of the new locking semantics.
BUG=None
TEST=devserver prevents concurrent access to the same staging location;
unit tests pass.
Change-Id: Icb8a20d475251b114ba632a040a7815eca395912
Reviewed-on: https://gerrit.chromium.org/gerrit/33139
Reviewed-by: Chris Sosa <sosa@chromium.org>
Commit-Ready: Gilad Arnold <garnold@chromium.org>
Tested-by: Gilad Arnold <garnold@chromium.org>
diff --git a/devserver_util.py b/devserver_util.py
index 8dc47b6..cf76bf8 100644
--- a/devserver_util.py
+++ b/devserver_util.py
@@ -7,6 +7,7 @@
import cherrypy
import distutils.version
import errno
+import lockfile
import os
import random
import re
@@ -21,6 +22,7 @@
MTON_DIR_SUFFIX = '_mton'
DEV_BUILD_PREFIX = 'dev'
UPLOADED_LIST = 'UPLOADED'
+DEVSERVER_LOCK_FILE = 'devserver'
class DevServerUtilError(Exception):
"""Exception classes used by this module."""
@@ -282,15 +284,18 @@
return (path.startswith(static_dir) and path != static_dir)
-def AcquireLock(static_dir, tag):
+def AcquireLock(static_dir, tag, create_once=True):
"""Acquires a lock for a given tag.
- Creates a directory for the specified tag, telling other
- components the resource/task represented by the tag is unavailable.
+ Creates a directory for the specified tag, and atomically creates a lock file
+ in it. This tells other components the resource/task represented by the tag
+ is unavailable.
Args:
- static_dir: Directory where builds are served from.
- tag: Unique resource/task identifier. Use '/' for nested tags.
+ static_dir: Directory where builds are served from.
+ tag: Unique resource/task identifier. Use '/' for nested tags.
+ create_once: Determines whether the directory must be freshly created; this
+ preserves previous semantics of the lock acquisition.
Returns:
Path to the created directory or None if creation failed.
@@ -302,23 +307,44 @@
if not SafeSandboxAccess(static_dir, build_dir):
raise DevServerUtilError('Invalid tag "%s".' % tag)
+ # Create the directory.
+ is_created = False
try:
os.makedirs(build_dir)
+ is_created = True
except OSError, e:
if e.errno == errno.EEXIST:
- raise DevServerUtilError(str(e))
+ if create_once:
+ raise DevServerUtilError(str(e))
else:
raise
+ # Lock the directory.
+ try:
+ lock = lockfile.FileLock(os.path.join(build_dir, DEVSERVER_LOCK_FILE))
+ lock.acquire(timeout=0)
+ except lockfile.AlreadyLocked, e:
+ raise DevServerUtilError(str(e))
+ except:
+ # In any other case, remove the directory if we actually created it, so
+ # that subsequent attempts won't fail to re-create it.
+ if is_created:
+ shutil.rmtree(build_dir)
+ raise
+
return build_dir
-def ReleaseLock(static_dir, tag):
- """Releases the lock for a given tag. Removes lock directory content.
+def ReleaseLock(static_dir, tag, destroy=False):
+ """Releases the lock for a given tag.
+
+ Optionally, removes the locked directory entirely.
Args:
static_dir: Directory where builds are served from.
- tag: Unique resource/task identifier. Use '/' for nested tags.
+ tag: Unique resource/task identifier. Use '/' for nested tags.
+ destroy: Determines whether the locked directory should be removed
+ entirely.
Raises:
DevServerUtilError: If lock can't be released.
@@ -327,7 +353,17 @@
if not SafeSandboxAccess(static_dir, build_dir):
raise DevServerUtilError('Invaid tag "%s".' % tag)
- shutil.rmtree(build_dir)
+ lock = lockfile.FileLock(os.path.join(build_dir, DEVSERVER_LOCK_FILE))
+ if lock.i_am_locking():
+ try:
+ lock.release()
+ if destroy:
+ shutil.rmtree(build_dir)
+ except Exception, e:
+ raise DevServerUtilError(str(e))
+ else:
+ raise DevServerUtilError('thread attempting release is not locking %s' %
+ build_dir)
def FindMatchingBoards(static_dir, board):
@@ -426,7 +462,7 @@
dev_build_exists = True
if force:
dev_build_exists = False
- ReleaseLock(dev_static_dir, tag)
+ ReleaseLock(dev_static_dir, tag, destroy=True)
AcquireLock(dev_static_dir, tag)
# Make a copy of the official build, only take necessary files.