Use file locks by default again

After a lot of discussion[1], it was decided that this is the
safest way forward.  In the future we can investigate alternative
locking methods, probably as opt-ins so we don't break any
existing consumers of these locks.

[1] http://lists.openstack.org/pipermail/openstack-dev/2014-August/043090.html

Change-Id: I49ff98abc395d3263d55aeff26081b462a8a294e
Partial-Bug: 1327946
This commit is contained in:
Ben Nemec 2014-08-21 22:16:15 +00:00
parent ca401cc68d
commit f1688a70f2
2 changed files with 22 additions and 79 deletions

View File

@ -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)

View File

@ -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: