diff --git a/oslo/concurrency/lockutils.py b/oslo/concurrency/lockutils.py index c852b5e..23bdc44 100644 --- a/oslo/concurrency/lockutils.py +++ b/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 - - import posix_ipc - InterProcessLock = _PosixLock - FileLock = _FcntlLock + InterProcessLock = _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 - # unnecessarily raise the RequiredOptError below. - if InterProcessLock is not _PosixLock: - raise cfg.RequiredOptError('lock_path') - local_lock_path = 'posixlock:/' + raise cfg.RequiredOptError('lock_path') 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) diff --git a/tests/unit/test_lockutils.py b/tests/unit/test_lockutils.py index 6cdc552..86593b1 100644 --- a/tests/unit/test_lockutils.py +++ b/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): - lock_name = 'a unique lock 123' - lock = lockutils.InterProcessLock(lock_name) + def test_lock_acquire_release_file_lock(self): + lock_dir = tempfile.mkdtemp() + 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.acquire(0) - my_lock.release() + my_lock = lockutils._FcntlLock(lock_file) + my_lock.lockfile = open(lock_file, 'w') + 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) - self.assertNotEqual(0, acquired_children) + try: + 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: