Allow breaking stale locks, refactor tests slightly.

Signed-off-by: Edward Z. Yang <ezyang@mit.edu>

diff --git a/TODO b/TODO
index c77331199181a91769a14561431b6696439e84f8..9cb904aba40c72794b2def2fa6bff1ae777b9e32 100644 (file)
--- a/TODO
+++ b/TODO
@@ -5,7 +5,6 @@ TODO NOW:
 - XXX: Upgrades don't pull updated tags, breaking git describe --tags!
   Fix this for the future, and figure out how to make everyone else happy!
 - XXX: Some installs are throwing spurious errors; investigate
-- XXX: Some installs are locked; we should automatically break locks if they're old
 - XXX: Prolly would be nice to have some information about how many installs actually succeeded
 - If you try to do an install on scripts w/o sql, it will sign you up but fail to write
   the sql.cnf file. This sucks.

diff --git a/wizard/tests/__init__.py b/wizard/tests/__init__.py
new file mode 100644 (file)
index 0000000..223b32c
--- /dev/null
+++ b/wizard/tests/__init__.py
@@ -0,0 +1,5 @@
+import os.path
+
+def getTestFile(file):
+    return os.path.join(os.path.dirname(os.path.abspath(__file__)), file)
+

diff --git a/wizard/tests/old_log_test.py b/wizard/tests/old_log_test.py
index 16e215c8d2e163da8f99f5ba9a53ce56bab2e4c1..75d9b5fb8c4ab8d2014c9bab2ae75b88499c6805 100644 (file)
--- a/wizard/tests/old_log_test.py
+++ b/wizard/tests/old_log_test.py
@@ -2,14 +2,11 @@ import os.path
 from dateutil.tz import tzoffset
 from datetime import datetime
 
-from wizard import app, deploy, old_log
-
-def getTestFile(file):
-    return os.path.join(os.path.dirname(os.path.abspath(__file__)), file)
+from wizard import app, deploy, old_log, tests
 
 def test_deploy_log_load():
     # this also is test_deploy_source_parse() and test_application_version_parse()
-    dlog = old_log.DeployLog.load(deploy.Deployment(getTestFile("old_log_test")))
+    dlog = old_log.DeployLog.load(deploy.Deployment(tests.getTestFile("old_log_test")))
 
     assert dlog[0].datetime == datetime(2006, 3, 23, 10, 7, 40, tzinfo=tzoffset(None, -5 * 60 * 60))
     assert dlog[0].user == "unknown"

diff --git a/wizard/tests/scripts_test.py b/wizard/tests/scripts_test.py
index d5a7abd1c300bbf3bc82acc90d92038a34c91d10..9cb120e1ebbd38e9d64a297abca77bf1b3f10fef 100644 (file)
--- a/wizard/tests/scripts_test.py
+++ b/wizard/tests/scripts_test.py
@@ -1,7 +1,9 @@
-from wizard.scripts import *
 import os
+import unittest
 
-testDirectory = os.path.dirname(os.path.abspath(__file__))
+from wizard import tests
+from wizard.scripts import *
 
-def test_get_disk_usage():
-    assert get_disk_usage(os.path.join(testDirectory, "scripts_test/"), "ignore_me") == 7
+class GetDiskUsageTest(unittest.TestCase):
+    def basicTest(self):
+        self.assertEqual(get_disk_usage(tests.getTestFile("scripts_test"), "ignore_me"), 7)

diff --git a/wizard/tests/util_test.py b/wizard/tests/util_test.py
index a328cd1516e2175d319d78ec6d7c7283eebed427..ba57792718bc55c76ca6e1d479d7c0c1d70a7e9a 100644 (file)
--- a/wizard/tests/util_test.py
+++ b/wizard/tests/util_test.py
@@ -1,7 +1,10 @@
 import traceback
 
+from wizard import tests
 from wizard.util import *
 
+lockfile = tests.getTestFile("util_test.lock")
+
 class MyError(Exception):
     def __str__(self):
         return """
@@ -54,3 +57,29 @@ def test_get_exception_name_withstr2():
         raise Exception("This is extra info we don't care about");
     except Exception:
         assert get_exception_name(traceback.format_exc()) == "Exception"
+
+def test_lock():
+    soft_unlink(lockfile)
+    with LockDirectory(lockfile):
+        pass
+
+def test_locked():
+    soft_unlink(lockfile)
+    with LockDirectory(lockfile):
+        try:
+            with LockDirectory(lockfile):
+                assert False
+        except DirectoryLockedError:
+            pass
+
+def test_break_orphan_lock():
+    soft_unlink(lockfile)
+    open(lockfile, "w").write("obviouslyboguspid")
+    with LockDirectory(lockfile):
+        pass
+
+def test_break_stale_lock():
+    soft_unlink(lockfile)
+    with LockDirectory(lockfile):
+        with LockDirectory(lockfile, expiry = 0):
+            pass

diff --git a/wizard/util.py b/wizard/util.py
index 174c13f161ed7524c4299fa7c5c0a59cb32eb240..a6e8f243dabf48a8792b442f51e75183ef4698c9 100644 (file)
--- a/wizard/util.py
+++ b/wizard/util.py
@@ -17,6 +17,8 @@ import itertools
 import signal
 import httplib
 import urllib
+import time
+import logging
 
 import wizard
 
@@ -96,17 +98,47 @@ class LockDirectory(object):
     """
     Context for locking a directory.
     """
-    def __init__(self, lockfile):
+    def __init__(self, lockfile, expiry = 3600):
         self.lockfile = lockfile
+        self.expiry = expiry # by default an hour
     def __enter__(self):
-        try:
-            os.open(self.lockfile, os.O_CREAT | os.O_EXCL)
-        except OSError as e:
-            if e.errno == errno.EEXIST:
-                raise DirectoryLockedError(os.getcwd())
-            elif e.errno == errno.EACCES:
-                raise PermissionsError(os.getcwd())
-            raise
+        # It's A WAVY
+        for i in range(0, 3):
+            try:
+                os.open(self.lockfile, os.O_CREAT | os.O_EXCL)
+                open(self.lockfile, "w").write("%d" % os.getpid())
+            except OSError as e:
+                if e.errno == errno.EEXIST:
+                    # There is a possibility of infinite recursion, but we
+                    # expect it to be unlikely, and not harmful if it does happen
+                    with LockDirectory(self.lockfile + "_"):
+                        # See if we can break the lock
+                        try:
+                            pid = open(self.lockfile, "r").read().strip()
+                            if not os.path.exists("/proc/%s" % pid):
+                                # break the lock, try again
+                                logging.warning("Breaking orphaned lock at %s", self.lockfile)
+                                os.unlink(self.lockfile)
+                                continue
+                            try:
+                                # check if the file is expiry old, if so, break the lock, try again
+                                if time.time() - os.stat(self.lockfile).st_mtime > self.expiry:
+                                    logging.warning("Breaking stale lock at %s", self.lockfile)
+                                    os.unlink(self.lockfile)
+                                    continue
+                            except OSError as e:
+                                if e.errno == errno.ENOENT:
+                                    continue
+                                raise
+                        except IOError:
+                            # oh hey, it went away; try again
+                            continue
+                    raise DirectoryLockedError(os.getcwd())
+                elif e.errno == errno.EACCES:
+                    raise PermissionsError(os.getcwd())
+                raise
+            return
+        raise DirectoryLockedError(os.getcwd())
     def __exit__(self, *args):
         try:
             os.unlink(self.lockfile)