Commit bug in synchronized where instance method lock on class if a synchronized class method had been called first.

docs/changes.rst

@@ -1,6 +1,15 @@
 Release Notes
 =============
 
+Version 1.5.1
+-------------
+
+**Bugs Fixed**
+
+* Instance method locking for the synchronized decorator was not correctly
+  locking on the instance but the class, if a synchronized class method
+  had been called prior to the synchronized instance method.
+
 Version 1.5.0
 -------------
 

src/decorators.py

@@ -248,7 +248,7 @@ def synchronized(wrapped):
     def _synchronized_lock(context):
         # Attempt to retrieve the lock for the specific context.
 
-        lock = getattr(context, '_synchronized_lock', None)
+        lock = vars(context).get('_synchronized_lock', None)
 
         if lock is None:
             # There is no existing lock defined for the context we
@@ -272,7 +272,7 @@ def synchronized(wrapped):
                 # at the same time and were competing to create the
                 # meta lock.
 
-                lock = getattr(context, '_synchronized_lock', None)
+                lock = vars(context).get('_synchronized_lock', None)
 
                 if lock is None:
                     lock = RLock()

tests/test_synchronized_lock.py

@@ -1,104 +1,36 @@
 from __future__ import print_function
 
 import unittest
-import threading
-import inspect
 
 import wrapt
 
-@wrapt.decorator
+@wrapt.synchronized
-def synchronized(wrapped, instance, args, kwargs):
-    if instance is None:
-        if inspect.isclass(wrapped):
-            # Wrapped function is a class and we are creating an
-            # instance of the class. Synchronisation is against
-            # the type.
-
-            context = wrapped
-            name = '_synchronized_type_lock'
-
-        else:
-            # Wrapped function is a normal function or static method.
-            # Synchronisation is against the individual function.
-
-            context = wrapped
-            name = '_synchronized_function_lock'
-
-    else:
-        if inspect.isclass(instance):
-            # Wrapped function is a class method. Synchronisation is
-            # against the class type.
-
-            context = instance
-            name = '_synchronized_class_lock'
-
-        else:
-            # Wrapped function is an instance method. Synchronisation
-            # is against the class instance.
-
-            context = instance
-            name = '_synchronized_instance_lock'
-
-    lock = getattr(context, name, None)
-
-    if lock is None:
-        # There is no existing lock defined for the context we
-        # are dealing with so we need to create one. This needs
-        # to be done in a way to guarantee there is only one
-        # created, even if multiple threads try and create it at
-        # the same time. We can't always use the setdefault()
-        # method on the __dict__ for the context. This is the
-        # case where the context is a class, as __dict__ is
-        # actually a dictproxy. What we therefore do is use a
-        # meta lock on this wrapper itself, to control the
-        # creation and assignment of the lock attribute against
-        # the context.
-
-        meta_lock = vars(synchronized).setdefault(
-                '_synchronized_meta_lock', threading.Lock())
-
-        with meta_lock:
-            # We need to check again for whether the lock we want
-            # exists in case two threads were trying to create it
-            # at the same time and were competing to create the
-            # meta lock.
-
-            lock = getattr(context, name, None)
-
-            if lock is None:
-                lock = threading.RLock()
-                setattr(context, name, lock)
-
-    with lock:
-        return wrapped(*args, **kwargs)
-
-@synchronized
 def function():
     print('function')
 
 class C1(object):
 
-    @synchronized
+    @wrapt.synchronized
     def function1(self):
         print('function1')
 
-    @synchronized
+    @wrapt.synchronized
     @classmethod
     def function2(cls):
         print('function2')
 
-    @synchronized
+    @wrapt.synchronized
     @staticmethod
     def function3():
         print('function3')
 
 c1 = C1()
 
-@synchronized
+@wrapt.synchronized
 class C2(object):
     pass
 
-@synchronized
+@wrapt.synchronized
 class C3:
     pass
 
@@ -112,12 +44,12 @@ class C4(object):
     # this.
 
     @classmethod
-    @synchronized
+    @wrapt.synchronized
     def function2(cls):
         print('function2')
 
     @staticmethod
-    @synchronized
+    @wrapt.synchronized
     def function3():
         print('function3')
 
@@ -126,86 +58,89 @@ c4 = C4()
 class TestSynchronized(unittest.TestCase):
 
     def test_synchronized_function(self):
-        _lock0 = getattr(function, '_synchronized_function_lock', None)
+        _lock0 = getattr(function, '_synchronized_lock', None)
         self.assertEqual(_lock0, None)
 
         function()
 
-        _lock1 = getattr(function, '_synchronized_function_lock', None)
+        _lock1 = getattr(function, '_synchronized_lock', None)
         self.assertNotEqual(_lock1, None)
 
         function()
 
-        _lock2 = getattr(function, '_synchronized_function_lock', None)
+        _lock2 = getattr(function, '_synchronized_lock', None)
         self.assertNotEqual(_lock2, None)
         self.assertEqual(_lock2, _lock1)
 
         function()
 
-        _lock3 = getattr(function, '_synchronized_function_lock', None)
+        _lock3 = getattr(function, '_synchronized_lock', None)
         self.assertNotEqual(_lock3, None)
         self.assertEqual(_lock3, _lock2)
 
     def test_synchronized_inner_staticmethod(self):
-        _lock0 = getattr(C1.function3, '_synchronized_function_lock', None)
+        _lock0 = getattr(C1.function3, '_synchronized_lock', None)
         self.assertEqual(_lock0, None)
 
         c1.function3()
 
-        _lock1 = getattr(C1.function3, '_synchronized_function_lock', None)
+        _lock1 = getattr(C1.function3, '_synchronized_lock', None)
         self.assertNotEqual(_lock1, None)
 
         C1.function3()
 
-        _lock2 = getattr(C1.function3, '_synchronized_function_lock', None)
+        _lock2 = getattr(C1.function3, '_synchronized_lock', None)
         self.assertNotEqual(_lock2, None)
         self.assertEqual(_lock2, _lock1)
 
         C1.function3()
 
-        _lock3 = getattr(C1.function3, '_synchronized_function_lock', None)
+        _lock3 = getattr(C1.function3, '_synchronized_lock', None)
         self.assertNotEqual(_lock3, None)
         self.assertEqual(_lock3, _lock2)
 
     def test_synchronized_outer_staticmethod(self):
-        _lock0 = getattr(C4.function3, '_synchronized_function_lock', None)
+        _lock0 = getattr(C4.function3, '_synchronized_lock', None)
         self.assertEqual(_lock0, None)
 
         c4.function3()
 
-        _lock1 = getattr(C4.function3, '_synchronized_function_lock', None)
+        _lock1 = getattr(C4.function3, '_synchronized_lock', None)
         self.assertNotEqual(_lock1, None)
 
         C4.function3()
 
-        _lock2 = getattr(C4.function3, '_synchronized_function_lock', None)
+        _lock2 = getattr(C4.function3, '_synchronized_lock', None)
         self.assertNotEqual(_lock2, None)
         self.assertEqual(_lock2, _lock1)
 
         C4.function3()
 
-        _lock3 = getattr(C4.function3, '_synchronized_function_lock', None)
+        _lock3 = getattr(C4.function3, '_synchronized_lock', None)
         self.assertNotEqual(_lock3, None)
         self.assertEqual(_lock3, _lock2)
 
     def test_synchronized_inner_classmethod(self):
-        _lock0 = getattr(C1, '_synchronized_class_lock', None)
+        if hasattr(C1, '_synchronized_lock'):
+            del C1._synchronized_lock
+
+        _lock0 = getattr(C1, '_synchronized_lock', None)
         self.assertEqual(_lock0, None)
 
         c1.function2()
 
-        _lock1 = getattr(C1, '_synchronized_class_lock', None)
+        _lock1 = getattr(C1, '_synchronized_lock', None)
         self.assertNotEqual(_lock1, None)
 
         C1.function2()
 
-        _lock2 = getattr(C1, '_synchronized_class_lock', None)
+        _lock2 = getattr(C1, '_synchronized_lock', None)
         self.assertNotEqual(_lock2, None)
         self.assertEqual(_lock2, _lock1)
 
         C1.function2()
 
-        _lock3 = getattr(C1, '_synchronized_class_lock', None)
+        _lock3 = getattr(C1, '_synchronized_lock', None)
         self.assertNotEqual(_lock3, None)
         self.assertEqual(_lock3, _lock2)
 
@@ -217,90 +152,112 @@ class TestSynchronized(unittest.TestCase):
         # first argument. This screws things up. Would be nice if
         # Python were fixed, but that isn't likely to happen.
 
-        #_lock0 = getattr(C4, '_synchronized_class_lock', None)
+        #_lock0 = getattr(C4, '_synchronized_lock', None)
-        _lock0 = getattr(C4.function2, '_synchronized_function_lock', None)
+        _lock0 = getattr(C4.function2, '_synchronized_lock', None)
         self.assertEqual(_lock0, None)
 
         c4.function2()
 
-        #_lock1 = getattr(C4, '_synchronized_class_lock', None)
+        #_lock1 = getattr(C4, '_synchronized_lock', None)
-        _lock1 = getattr(C4.function2, '_synchronized_function_lock', None)
+        _lock1 = getattr(C4.function2, '_synchronized_lock', None)
         self.assertNotEqual(_lock1, None)
 
         C4.function2()
 
-        #_lock2 = getattr(C4, '_synchronized_class_lock', None)
+        #_lock2 = getattr(C4, '_synchronized_lock', None)
-        _lock2 = getattr(C4.function2, '_synchronized_function_lock', None)
+        _lock2 = getattr(C4.function2, '_synchronized_lock', None)
         self.assertNotEqual(_lock2, None)
         self.assertEqual(_lock2, _lock1)
 
         C4.function2()
 
-        #_lock3 = getattr(C4, '_synchronized_class_lock', None)
+        #_lock3 = getattr(C4, '_synchronized_lock', None)
-        _lock3 = getattr(C4.function2, '_synchronized_function_lock', None)
+        _lock3 = getattr(C4.function2, '_synchronized_lock', None)
         self.assertNotEqual(_lock3, None)
         self.assertEqual(_lock3, _lock2)
 
-    def test_syncrhonized_instancemethod(self):
+    def test_synchronized_instancemethod(self):
-        _lock0 = getattr(c1, '_synchronized_instance_lock', None)
+        if hasattr(C1, '_synchronized_lock'):
+            del C1._synchronized_lock
+
+        _lock0 = getattr(c1, '_synchronized_lock', None)
         self.assertEqual(_lock0, None)
 
         C1.function1(c1)
 
-        _lock1 = getattr(c1, '_synchronized_instance_lock', None)
+        _lock1 = getattr(c1, '_synchronized_lock', None)
         self.assertNotEqual(_lock1, None)
 
         c1.function1()
 
-        _lock2 = getattr(c1, '_synchronized_instance_lock', None)
+        _lock2 = getattr(c1, '_synchronized_lock', None)
         self.assertNotEqual(_lock2, None)
         self.assertEqual(_lock2, _lock1)
 
         c1.function1()
 
-        _lock3 = getattr(c1, '_synchronized_instance_lock', None)
+        _lock3 = getattr(c1, '_synchronized_lock', None)
         self.assertNotEqual(_lock3, None)
         self.assertEqual(_lock3, _lock2)
 
-    def test_syncrhonized_type(self):
+        del c1._synchronized_lock
-        _lock0 = getattr(C2, '_synchronized_type_lock', None)
+
+        C1.function2()
+
+        _lock4 = getattr(C1, '_synchronized_lock', None)
+        self.assertNotEqual(_lock4, None)
+
+        c1.function1()
+
+        _lock5 = getattr(c1, '_synchronized_lock', None)
+        self.assertNotEqual(_lock5, None)
+        self.assertNotEqual(_lock5, _lock4)
+
+    def test_synchronized_type_new_style(self):
+        if hasattr(C2, '_synchronized_lock'):
+            del C2._synchronized_lock
+
+        _lock0 = getattr(C2, '_synchronized_lock', None)
         self.assertEqual(_lock0, None)
 
         c2 = C2()
 
-        _lock1 = getattr(C2, '_synchronized_type_lock', None)
+        _lock1 = getattr(C2, '_synchronized_lock', None)
         self.assertNotEqual(_lock1, None)
 
         c2 = C2()
 
-        _lock2 = getattr(C2, '_synchronized_type_lock', None)
+        _lock2 = getattr(C2, '_synchronized_lock', None)
         self.assertNotEqual(_lock2, None)
         self.assertEqual(_lock2, _lock1)
 
         c2 = C2()
 
-        _lock3 = getattr(C2, '_synchronized_type_lock', None)
+        _lock3 = getattr(C2, '_synchronized_lock', None)
         self.assertNotEqual(_lock3, None)
         self.assertEqual(_lock3, _lock2)
 
-    def test_syncrhonized_type_old_style(self):
+    def test_synchronized_type_old_style(self):
-        _lock0 = getattr(C3, '_synchronized_type_lock', None)
+        if hasattr(C3, '_synchronized_lock'):
+            del C3._synchronized_lock
+
+        _lock0 = getattr(C3, '_synchronized_lock', None)
         self.assertEqual(_lock0, None)
 
         c2 = C3()
 
-        _lock1 = getattr(C3, '_synchronized_type_lock', None)
+        _lock1 = getattr(C3, '_synchronized_lock', None)
         self.assertNotEqual(_lock1, None)
 
         c2 = C3()
 
-        _lock2 = getattr(C3, '_synchronized_type_lock', None)
+        _lock2 = getattr(C3, '_synchronized_lock', None)
         self.assertNotEqual(_lock2, None)
         self.assertEqual(_lock2, _lock1)
 
         c2 = C3()
 
-        _lock3 = getattr(C3, '_synchronized_type_lock', None)
+        _lock3 = getattr(C3, '_synchronized_lock', None)
         self.assertNotEqual(_lock3, None)
         self.assertEqual(_lock3, _lock2)