commit 5174ca2e6d84adc8ddbc21a62aee2dfb3f48fc2f
author Gilad Arnold <garnold@chromium.org> Wed Sep 12 10:49:09 2012 -0700
committer Gerrit <chrome-bot@google.com> Fri Sep 21 12:29:58 2012 -0700
tree 903bdf683d7d33c6e738d1b68326450029685c65
parent 468d87593f9612aaf05c1dd023c8874dcfbb1f7f

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>

downloader.py

diff --git a/downloader.py b/downloader.py
index 710a425..05f87bb 100755
--- a/downloader.py
+++ b/downloader.py
@@ -105,7 +105,9 @@
     self._lock_tag = self.GenerateLockTag(rel_path, short_build)
     try:
       # Create Dev Server directory for this build and tell other Downloader
-      # instances we have processed this build.
+      # instances we have processed this build. Note that during normal
+      # execution, this lock is only released in the actual downloading
+      # procedure called below.
       self._build_dir = devserver_util.AcquireLock(
           static_dir=self._static_dir, tag=self._lock_tag)
 
@@ -140,12 +142,11 @@
       # so future runs can retry.
       if self._build_dir:
         devserver_util.ReleaseLock(static_dir=self._static_dir,
-                                   tag=self._lock_tag)
+                                   tag=self._lock_tag, destroy=True)
 
       self._status_queue.put(e)
       self._Cleanup()
       raise
-
     return 'Success'
 
   def _Cleanup(self):
@@ -171,8 +172,12 @@
       # so future runs can retry.
       if self._build_dir:
         devserver_util.ReleaseLock(static_dir=self._static_dir,
-                                   tag=self._lock_tag)
+                                   tag=self._lock_tag, destroy=True)
     else:
+      # Release processing lock, keeping directory intact.
+      if self._build_dir:
+        devserver_util.ReleaseLock(static_dir=self._static_dir,
+                                   tag=self._lock_tag)
       self._status_queue.put('Success')
     finally:
       self._Cleanup()
@@ -273,8 +278,14 @@
       # did not succeed, and so they should try again.
       if self._build_dir:
         devserver_util.ReleaseLock(static_dir=self._static_dir,
-                                   tag=self._lock_tag)
+                                   tag=self._lock_tag, destroy=True)
+
       raise
+    else:
+      # Release processing "lock", keeping directory intact.
+      if self._build_dir:
+        devserver_util.ReleaseLock(static_dir=self._static_dir,
+                                   tag=self._lock_tag)
     finally:
       self._Cleanup()