Merge "Use file locks by default again"

oslo/concurrency/lockutils.py

@@ -148,58 +148,12 @@ class _FcntlLock(_FileLock):
         fcntl.lockf(self.lockfile, fcntl.LOCK_UN)
 
 
-class _PosixLock(object):
-    def __init__(self, name):
-        # Hash the name because it's not valid to have POSIX semaphore
-        # names with things like / in them. Then use base64 to encode
-        # the digest() instead taking the hexdigest() because the
-        # result is shorter and most systems can't have shm sempahore
-        # names longer than 31 characters.
-        h = hashlib.sha1()
-        h.update(name.encode('ascii'))
-        self.name = str((b'/' + base64.urlsafe_b64encode(
-            h.digest())).decode('ascii'))
-
-    def acquire(self, timeout=None):
-        self.semaphore = posix_ipc.Semaphore(self.name,
-                                             flags=posix_ipc.O_CREAT,
-                                             initial_value=1)
-        self.semaphore.acquire(timeout)
-        return self
-
-    def __enter__(self):
-        self.acquire()
-        return self
-
-    def release(self):
-        self.semaphore.release()
-        self.semaphore.close()
-
-    def __exit__(self, exc_type, exc_val, exc_tb):
-        self.release()
-
-    def exists(self):
-        try:
-            semaphore = posix_ipc.Semaphore(self.name)
-        except posix_ipc.ExistentialError:
-            return False
-        else:
-            semaphore.close()
-        return True
-
-
 if os.name == 'nt':
     import msvcrt
     InterProcessLock = _WindowsLock
-    FileLock = _WindowsLock
 else:
-    import base64
     import fcntl
-    import hashlib
+    InterProcessLock = _FcntlLock
-
-    import posix_ipc
-    InterProcessLock = _PosixLock
-    FileLock = _FcntlLock
 
 _semaphores = weakref.WeakValueDictionary()
 _semaphores_lock = threading.Lock()
@@ -216,11 +170,7 @@ def _get_lock_path(name, lock_file_prefix, lock_path=None):
     local_lock_path = lock_path or CONF.lock_path
 
     if not local_lock_path:
-        # NOTE(bnemec): Create a fake lock path for posix locks so we don't
+        raise cfg.RequiredOptError('lock_path')
-        # unnecessarily raise the RequiredOptError below.
-        if InterProcessLock is not _PosixLock:
-            raise cfg.RequiredOptError('lock_path')
-        local_lock_path = 'posixlock:/'
 
     return os.path.join(local_lock_path, name)
 
@@ -231,11 +181,6 @@ def external_lock(name, lock_file_prefix=None, lock_path=None):
 
     lock_file_path = _get_lock_path(name, lock_file_prefix, lock_path)
 
-    # NOTE(bnemec): If an explicit lock_path was passed to us then it
-    # means the caller is relying on file-based locking behavior, so
-    # we can't use posix locks for those calls.
-    if lock_path:
-        return FileLock(lock_file_path)
     return InterProcessLock(lock_file_path)
 
 

tests/unit/test_lockutils.py

@@ -49,17 +49,20 @@ class LockTestCase(test_base.BaseTestCase):
         self.assertEqual(foo.__name__, 'foo', "Wrapped function's name "
                                               "got mangled")
 
-    def test_lock_acquire_release(self):
+    def test_lock_acquire_release_file_lock(self):
-        lock_name = 'a unique lock 123'
+        lock_dir = tempfile.mkdtemp()
-        lock = lockutils.InterProcessLock(lock_name)
+        lock_file = os.path.join(lock_dir, 'lock')
+        lock = lockutils._FcntlLock(lock_file)
 
         def try_lock():
+            lock.release()  # child co-owns it before fork
             try:
-                my_lock = lockutils.InterProcessLock(lock_name)
+                my_lock = lockutils._FcntlLock(lock_file)
-                my_lock.acquire(0)
+                my_lock.lockfile = open(lock_file, 'w')
-                my_lock.release()
+                my_lock.trylock()
+                my_lock.unlock()
                 os._exit(1)
-            except Exception:
+            except IOError:
                 os._exit(0)
 
         def attempt_acquire(count):
@@ -81,8 +84,14 @@ class LockTestCase(test_base.BaseTestCase):
         finally:
             lock.release()
 
-        acquired_children = attempt_acquire(5)
+        try:
-        self.assertNotEqual(0, acquired_children)
+            acquired_children = attempt_acquire(5)
+            self.assertNotEqual(0, acquired_children)
+        finally:
+            try:
+                shutil.rmtree(lock_dir)
+            except IOError:
+                pass
 
     def test_lock_internally(self):
         """We can lock across multiple threads."""
@@ -362,17 +371,6 @@ class FileBasedLockingTestCase(test_base.BaseTestCase):
 
         self.assertRaises(threading.ThreadError, lock.acquire)
 
-    def test_no_lock_path(self):
-        lock_file = os.path.join(self.lock_dir, 'should-not-exist')
-
-        @lockutils.synchronized('should-not-exist', external=True)
-        def foo():
-            # Without lock_path explicitly passed to synchronized, we should
-            # default to using posix locks and not create a lock file.
-            self.assertFalse(os.path.exists(lock_file))
-
-        foo()
-
     def test_interprocess_lock(self):
         lock_file = os.path.join(self.lock_dir, 'processlock')
 
@@ -384,12 +382,12 @@ class FileBasedLockingTestCase(test_base.BaseTestCase):
                 if time.time() - start > 5:
                     self.fail('Timed out waiting for child to grab lock')
                 time.sleep(0)
-            lock1 = lockutils.FileLock('foo')
+            lock1 = lockutils.InterProcessLock('foo')
             lock1.lockfile = open(lock_file, 'w')
             self.assertRaises(IOError, lock1.trylock)
         else:
             try:
-                lock2 = lockutils.FileLock('foo')
+                lock2 = lockutils.InterProcessLock('foo')
                 lock2.lockfile = open(lock_file, 'w')
                 lock2.trylock()
             finally: