Merge "Move synchronized body to a first-class function"
This commit is contained in:
Jenkins
Gerrit Code Review
commit
0ae9ebb507
|
|
@ -16,6 +16,7 @@
|
|||
# under the License.
|
||||
|
||||
|
||||
import contextlib
|
||||
import errno
|
||||
import functools
|
||||
import os
|
||||
|
|
@ -135,28 +136,12 @@ else:
|
|||
_semaphores = weakref.WeakValueDictionary()
|
||||
|
||||
|
||||
def synchronized(name, lock_file_prefix=None, external=False, lock_path=None):
|
||||
"""Synchronization decorator.
|
||||
@contextlib.contextmanager
|
||||
def lock(name, lock_file_prefix=None, external=False, lock_path=None):
|
||||
"""Context based lock
|
||||
|
||||
Decorating a method like so::
|
||||
|
||||
@synchronized('mylock')
|
||||
def foo(self, *args):
|
||||
...
|
||||
|
||||
ensures that only one thread will execute the foo method at a time.
|
||||
|
||||
Different methods can share the same lock::
|
||||
|
||||
@synchronized('mylock')
|
||||
def foo(self, *args):
|
||||
...
|
||||
|
||||
@synchronized('mylock')
|
||||
def bar(self, *args):
|
||||
...
|
||||
|
||||
This way only one of either foo or bar can be executing at a time.
|
||||
This function yields a `semaphore.Semaphore` instance unless external is
|
||||
True, in which case, it'll yield an InterProcessLock instance.
|
||||
|
||||
:param lock_file_prefix: The lock_file_prefix argument is used to provide
|
||||
lock files on disk with a meaningful prefix.
|
||||
|
|
@ -170,10 +155,6 @@ def synchronized(name, lock_file_prefix=None, external=False, lock_path=None):
|
|||
special location for external lock files to live. If nothing is set, then
|
||||
CONF.lock_path is used as a default.
|
||||
"""
|
||||
|
||||
def wrap(f):
|
||||
@functools.wraps(f)
|
||||
def inner(*args, **kwargs):
|
||||
# NOTE(soren): If we ever go natively threaded, this will be racy.
|
||||
# See http://stackoverflow.com/questions/5390569/dyn
|
||||
# amically-allocating-and-destroying-mutexes
|
||||
|
|
@ -185,9 +166,7 @@ def synchronized(name, lock_file_prefix=None, external=False, lock_path=None):
|
|||
_semaphores[name] = sem
|
||||
|
||||
with sem:
|
||||
LOG.debug(_('Got semaphore "%(lock)s" for method '
|
||||
'"%(method)s"...'), {'lock': name,
|
||||
'method': f.__name__})
|
||||
LOG.debug(_('Got semaphore "%(lock)s"'), {'lock': name})
|
||||
|
||||
# NOTE(mikal): I know this looks odd
|
||||
if not hasattr(local.strong_store, 'locks_held'):
|
||||
|
|
@ -196,9 +175,8 @@ def synchronized(name, lock_file_prefix=None, external=False, lock_path=None):
|
|||
|
||||
try:
|
||||
if external and not CONF.disable_process_locking:
|
||||
LOG.debug(_('Attempting to grab file lock "%(lock)s" '
|
||||
'for method "%(method)s"...'),
|
||||
{'lock': name, 'method': f.__name__})
|
||||
LOG.debug(_('Attempting to grab file lock "%(lock)s"'),
|
||||
{'lock': name})
|
||||
cleanup_dir = False
|
||||
|
||||
# We need a copy of lock_path because it is non-local
|
||||
|
|
@ -224,25 +202,17 @@ def synchronized(name, lock_file_prefix=None, external=False, lock_path=None):
|
|||
lock_file_name = add_prefix(name.replace(os.sep, '_'),
|
||||
lock_file_prefix)
|
||||
|
||||
lock_file_path = os.path.join(local_lock_path,
|
||||
lock_file_name)
|
||||
lock_file_path = os.path.join(local_lock_path, lock_file_name)
|
||||
|
||||
try:
|
||||
lock = InterProcessLock(lock_file_path)
|
||||
with lock:
|
||||
LOG.debug(_('Got file lock "%(lock)s" at '
|
||||
'%(path)s for method '
|
||||
'"%(method)s"...'),
|
||||
{'lock': name,
|
||||
'path': lock_file_path,
|
||||
'method': f.__name__})
|
||||
retval = f(*args, **kwargs)
|
||||
with lock as lock:
|
||||
LOG.debug(_('Got file lock "%(lock)s" at %(path)s'),
|
||||
{'lock': name, 'path': lock_file_path})
|
||||
yield lock
|
||||
finally:
|
||||
LOG.debug(_('Released file lock "%(lock)s" at '
|
||||
'%(path)s for method "%(method)s"...'),
|
||||
{'lock': name,
|
||||
'path': lock_file_path,
|
||||
'method': f.__name__})
|
||||
LOG.debug(_('Released file lock "%(lock)s" at %(path)s'),
|
||||
{'lock': name, 'path': lock_file_path})
|
||||
# NOTE(vish): This removes the tempdir if we needed
|
||||
# to create one. This is used to
|
||||
# cleanup the locks left behind by unit
|
||||
|
|
@ -250,12 +220,46 @@ def synchronized(name, lock_file_prefix=None, external=False, lock_path=None):
|
|||
if cleanup_dir:
|
||||
shutil.rmtree(local_lock_path)
|
||||
else:
|
||||
retval = f(*args, **kwargs)
|
||||
yield sem
|
||||
|
||||
finally:
|
||||
local.strong_store.locks_held.remove(name)
|
||||
|
||||
return retval
|
||||
|
||||
def synchronized(name, lock_file_prefix=None, external=False, lock_path=None):
|
||||
"""Synchronization decorator.
|
||||
|
||||
Decorating a method like so::
|
||||
|
||||
@synchronized('mylock')
|
||||
def foo(self, *args):
|
||||
...
|
||||
|
||||
ensures that only one thread will execute the foo method at a time.
|
||||
|
||||
Different methods can share the same lock::
|
||||
|
||||
@synchronized('mylock')
|
||||
def foo(self, *args):
|
||||
...
|
||||
|
||||
@synchronized('mylock')
|
||||
def bar(self, *args):
|
||||
...
|
||||
|
||||
This way only one of either foo or bar can be executing at a time.
|
||||
"""
|
||||
|
||||
def wrap(f):
|
||||
@functools.wraps(f)
|
||||
def inner(*args, **kwargs):
|
||||
with lock(name, lock_file_prefix, external, lock_path):
|
||||
LOG.debug(_('Got semaphore / lock "%(function)s"'),
|
||||
{'function': f.__name__})
|
||||
return f(*args, **kwargs)
|
||||
|
||||
LOG.debug(_('Semaphore / lock released "%(function)s"'),
|
||||
{'function': f.__name__})
|
||||
return inner
|
||||
return wrap
|
||||
|
||||
|
|
|
|||
|
|
@ -22,12 +22,14 @@ import tempfile
|
|||
import eventlet
|
||||
from eventlet import greenpool
|
||||
from eventlet import greenthread
|
||||
from eventlet import semaphore
|
||||
|
||||
from openstack.common import lockutils
|
||||
from tests import utils
|
||||
|
||||
|
||||
class TestFileLocks(utils.BaseTestCase):
|
||||
|
||||
def test_concurrent_green_lock_succeeds(self):
|
||||
"""Verify spawn_n greenthreads with two locks run concurrently."""
|
||||
tmpdir = tempfile.mkdtemp()
|
||||
|
|
@ -63,6 +65,7 @@ class TestFileLocks(utils.BaseTestCase):
|
|||
|
||||
|
||||
class LockTestCase(utils.BaseTestCase):
|
||||
|
||||
def test_synchronized_wrapped_function_metadata(self):
|
||||
@lockutils.synchronized('whatever', 'test-')
|
||||
def foo():
|
||||
|
|
@ -74,15 +77,15 @@ class LockTestCase(utils.BaseTestCase):
|
|||
self.assertEquals(foo.__name__, 'foo', "Wrapped function's name "
|
||||
"got mangled")
|
||||
|
||||
def test_synchronized_internally(self):
|
||||
def test_lock_internally(self):
|
||||
"""We can lock across multiple green threads."""
|
||||
saved_sem_num = len(lockutils._semaphores)
|
||||
seen_threads = list()
|
||||
|
||||
@lockutils.synchronized('testlock2', 'test-', external=False)
|
||||
def f(id):
|
||||
def f(_id):
|
||||
with lockutils.lock('testlock2', 'test-', external=False):
|
||||
for x in range(10):
|
||||
seen_threads.append(id)
|
||||
seen_threads.append(_id)
|
||||
greenthread.sleep(0)
|
||||
|
||||
threads = []
|
||||
|
|
@ -104,7 +107,7 @@ class LockTestCase(utils.BaseTestCase):
|
|||
self.assertEqual(saved_sem_num, len(lockutils._semaphores),
|
||||
"Semaphore leak detected")
|
||||
|
||||
def test_nested_external_works(self):
|
||||
def test_nested_synchronized_external_works(self):
|
||||
"""We can nest external syncs."""
|
||||
tempdir = tempfile.mkdtemp()
|
||||
try:
|
||||
|
|
@ -125,11 +128,12 @@ class LockTestCase(utils.BaseTestCase):
|
|||
if os.path.exists(tempdir):
|
||||
shutil.rmtree(tempdir)
|
||||
|
||||
def _do_test_synchronized_externally(self):
|
||||
def _do_test_lock_externally(self):
|
||||
"""We can lock across multiple processes."""
|
||||
|
||||
@lockutils.synchronized('external', 'test-', external=True)
|
||||
def lock_files(handles_dir):
|
||||
|
||||
with lockutils.lock('external', 'test-', external=True):
|
||||
# Open some files we can use for locking
|
||||
handles = []
|
||||
for n in range(50):
|
||||
|
|
@ -175,23 +179,23 @@ class LockTestCase(utils.BaseTestCase):
|
|||
if os.path.exists(handles_dir):
|
||||
shutil.rmtree(handles_dir, ignore_errors=True)
|
||||
|
||||
def test_synchronized_externally(self):
|
||||
def test_lock_externally(self):
|
||||
lock_dir = tempfile.mkdtemp()
|
||||
self.config(lock_path=lock_dir)
|
||||
|
||||
try:
|
||||
self._do_test_synchronized_externally()
|
||||
self._do_test_lock_externally()
|
||||
finally:
|
||||
if os.path.exists(lock_dir):
|
||||
shutil.rmtree(lock_dir, ignore_errors=True)
|
||||
|
||||
def test_synchronized_externally_lock_dir_not_exist(self):
|
||||
def test_lock_externally_lock_dir_not_exist(self):
|
||||
lock_dir = tempfile.mkdtemp()
|
||||
os.rmdir(lock_dir)
|
||||
self.config(lock_path=lock_dir)
|
||||
|
||||
try:
|
||||
self._do_test_synchronized_externally()
|
||||
self._do_test_lock_externally()
|
||||
finally:
|
||||
if os.path.exists(lock_dir):
|
||||
shutil.rmtree(lock_dir, ignore_errors=True)
|
||||
|
|
@ -239,3 +243,56 @@ class LockTestCase(utils.BaseTestCase):
|
|||
finally:
|
||||
if os.path.exists(lock_dir):
|
||||
shutil.rmtree(lock_dir, ignore_errors=True)
|
||||
|
||||
def test_contextlock(self):
|
||||
lock_dir = tempfile.mkdtemp()
|
||||
|
||||
try:
|
||||
# Note(flaper87): Lock is not external, which means
|
||||
# a semaphore will be yielded
|
||||
with lockutils.lock("test") as sem:
|
||||
self.assertTrue(isinstance(sem, semaphore.Semaphore))
|
||||
|
||||
# NOTE(flaper87): Lock is external so an InterProcessLock
|
||||
# will be yielded.
|
||||
with lockutils.lock("test2", external=True,
|
||||
lock_path=lock_dir):
|
||||
path = os.path.join(lock_dir, "test2")
|
||||
self.assertTrue(os.path.exists(path))
|
||||
|
||||
with lockutils.lock("test1",
|
||||
external=True,
|
||||
lock_path=lock_dir) as lock1:
|
||||
self.assertTrue(isinstance(lock1,
|
||||
lockutils.InterProcessLock))
|
||||
finally:
|
||||
if os.path.exists(lock_dir):
|
||||
shutil.rmtree(lock_dir, ignore_errors=True)
|
||||
|
||||
def test_contextlock_unlocks(self):
|
||||
lock_dir = tempfile.mkdtemp()
|
||||
|
||||
sem = None
|
||||
|
||||
try:
|
||||
with lockutils.lock("test") as sem:
|
||||
self.assertTrue(isinstance(sem, semaphore.Semaphore))
|
||||
|
||||
with lockutils.lock("test2", external=True,
|
||||
lock_path=lock_dir):
|
||||
path = os.path.join(lock_dir, "test2")
|
||||
self.assertTrue(os.path.exists(path))
|
||||
|
||||
# NOTE(flaper87): Lock should be free
|
||||
with lockutils.lock("test2", external=True,
|
||||
lock_path=lock_dir):
|
||||
path = os.path.join(lock_dir, "test2")
|
||||
self.assertTrue(os.path.exists(path))
|
||||
|
||||
# NOTE(flaper87): Lock should be free
|
||||
# but semaphore should already exist.
|
||||
with lockutils.lock("test") as sem2:
|
||||
self.assertEqual(sem, sem2)
|
||||
finally:
|
||||
if os.path.exists(lock_dir):
|
||||
shutil.rmtree(lock_dir, ignore_errors=True)
|
||||
|
|
|
|||
Loading…
Reference in New Issue