Merge "Move synchronized body to a first-class function"

1 files changed, 97 insertions, 93 deletions

diff --git a/openstack/common/lockutils.py b/openstack/common/lockutils.py
index ad8c0e5..01f6c94 100644
--- a/openstack/common/lockutils.py
+++ b/openstack/common/lockutils.py
@@ -16,6 +16,7 @@
 #    under the License.
 
 
+import contextlib
 import errno
 import functools
 import os
@@ -135,6 +136,96 @@ else:
 _semaphores = weakref.WeakValueDictionary()
 
 
+@contextlib.contextmanager
+def lock(name, lock_file_prefix=None, external=False, lock_path=None):
+    """Context based lock
+
+    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.
+
+    :param external: The external keyword argument denotes whether this lock
+    should work across multiple processes. This means that if two different
+    workers both run a a method decorated with @synchronized('mylock',
+    external=True), only one of them will execute at a time.
+
+    :param lock_path: The lock_path keyword argument is used to specify a
+    special location for external lock files to live. If nothing is set, then
+    CONF.lock_path is used as a default.
+    """
+    # NOTE(soren): If we ever go natively threaded, this will be racy.
+    #              See http://stackoverflow.com/questions/5390569/dyn
+    #              amically-allocating-and-destroying-mutexes
+    sem = _semaphores.get(name, semaphore.Semaphore())
+    if name not in _semaphores:
+        # this check is not racy - we're already holding ref locally
+        # so GC won't remove the item and there was no IO switch
+        # (only valid in greenthreads)
+        _semaphores[name] = sem
+
+    with sem:
+        LOG.debug(_('Got semaphore "%(lock)s"'), {'lock': name})
+
+        # NOTE(mikal): I know this looks odd
+        if not hasattr(local.strong_store, 'locks_held'):
+            local.strong_store.locks_held = []
+        local.strong_store.locks_held.append(name)
+
+        try:
+            if external and not CONF.disable_process_locking:
+                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
+                local_lock_path = lock_path
+                if not local_lock_path:
+                    local_lock_path = CONF.lock_path
+
+                if not local_lock_path:
+                    cleanup_dir = True
+                    local_lock_path = tempfile.mkdtemp()
+
+                if not os.path.exists(local_lock_path):
+                    fileutils.ensure_tree(local_lock_path)
+
+                def add_prefix(name, prefix):
+                    if not prefix:
+                        return name
+                    sep = '' if prefix.endswith('-') else '-'
+                    return '%s%s%s' % (prefix, sep, name)
+
+                # NOTE(mikal): the lock name cannot contain directory
+                # separators
+                lock_file_name = add_prefix(name.replace(os.sep, '_'),
+                                            lock_file_prefix)
+
+                lock_file_path = os.path.join(local_lock_path, lock_file_name)
+
+                try:
+                    lock = InterProcessLock(lock_file_path)
+                    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'),
+                              {'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
+                    #             tests.
+                    if cleanup_dir:
+                        shutil.rmtree(local_lock_path)
+            else:
+                yield sem
+
+        finally:
+            local.strong_store.locks_held.remove(name)
+
+
 def synchronized(name, lock_file_prefix=None, external=False, lock_path=None):
     """Synchronization decorator.
 
@@ -157,105 +248,18 @@ def synchronized(name, lock_file_prefix=None, external=False, lock_path=None):
            ...
 
     This way only one of either foo or bar can be executing at a time.
-
-    :param lock_file_prefix: The lock_file_prefix argument is used to provide
-    lock files on disk with a meaningful prefix.
-
-    :param external: The external keyword argument denotes whether this lock
-    should work across multiple processes. This means that if two different
-    workers both run a a method decorated with @synchronized('mylock',
-    external=True), only one of them will execute at a time.
-
-    :param lock_path: The lock_path keyword argument is used to specify a
-    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
-            sem = _semaphores.get(name, semaphore.Semaphore())
-            if name not in _semaphores:
-                # this check is not racy - we're already holding ref locally
-                # so GC won't remove the item and there was no IO switch
-                # (only valid in greenthreads)
-                _semaphores[name] = sem
-
-            with sem:
-                LOG.debug(_('Got semaphore "%(lock)s" for method '
-                            '"%(method)s"...'), {'lock': name,
-                                                 'method': f.__name__})
-
-                # NOTE(mikal): I know this looks odd
-                if not hasattr(local.strong_store, 'locks_held'):
-                    local.strong_store.locks_held = []
-                local.strong_store.locks_held.append(name)
-
-                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__})
-                        cleanup_dir = False
-
-                        # We need a copy of lock_path because it is non-local
-                        local_lock_path = lock_path
-                        if not local_lock_path:
-                            local_lock_path = CONF.lock_path
-
-                        if not local_lock_path:
-                            cleanup_dir = True
-                            local_lock_path = tempfile.mkdtemp()
-
-                        if not os.path.exists(local_lock_path):
-                            fileutils.ensure_tree(local_lock_path)
-
-                        def add_prefix(name, prefix):
-                            if not prefix:
-                                return name
-                            sep = '' if prefix.endswith('-') else '-'
-                            return '%s%s%s' % (prefix, sep, name)
-
-                        # NOTE(mikal): the lock name cannot contain directory
-                        # separators
-                        lock_file_name = add_prefix(name.replace(os.sep, '_'),
-                                                    lock_file_prefix)
-
-                        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)
-                        finally:
-                            LOG.debug(_('Released file lock "%(lock)s" at '
-                                        '%(path)s for method "%(method)s"...'),
-                                      {'lock': name,
-                                       'path': lock_file_path,
-                                       'method': f.__name__})
-                            # NOTE(vish): This removes the tempdir if we needed
-                            #             to create one. This is used to
-                            #             cleanup the locks left behind by unit
-                            #             tests.
-                            if cleanup_dir:
-                                shutil.rmtree(local_lock_path)
-                    else:
-                        retval = f(*args, **kwargs)
-
-                finally:
-                    local.strong_store.locks_held.remove(name)
+            with lock(name, lock_file_prefix, external, lock_path):
+                LOG.debug(_('Got semaphore / lock "%(function)s"'),
+                          {'function': f.__name__})
+                return f(*args, **kwargs)
 
-            return retval
+            LOG.debug(_('Semaphore / lock released "%(function)s"'),
+                      {'function': f.__name__})
         return inner
     return wrap