Metaclasses, decorators, and synchronization

[Scott David Daniels]

Michael Ekstrand wrote:
> Something like this (not yet tested):
> 
> import threading
> global_lock = threading.Lock()
> def synchronized(meth):
>     def inner(self, *args, **kwargs):
>         try:
>             self._sync_lock.acquire()
>         except AttributeError:
>             global_lock.acquire()
>             if not hasattr(self, '_sync_lock'):
>                 self._sync_lock = threading.RLock()
>             self._sync_lock.acquire()
>             global_lock.release()
>         meth(self, *args, **kwargs)
>         self._sync_lock.release()
>     return inner
> I don't think this solution has any race conditions (hence the re-check 
> of the existence of _sync_lock), and I've tried to optimize it for the 
> common case in my project (calling the method after the lock has been 
> put in place - that will happen much more than a call to a synchronized 
> method of a fresh object).

Better would probably be:

 >         ...
 >         except AttributeError:
 >             global_lock.acquire()
 >             if not hasattr(self, '_sync_lock'):
 >                 self._sync_lock = threading.RLock()
 >             global_lock.release()  # release first
 >             self._sync_lock.acquire()
 >         ...
or even:
 > def synchronized(meth):
 >     def inner(self, *args, **kwargs):
 >         try:
 >             self._sync_lock.acquire()
 >         except AttributeError:
 >             global_lock.acquire()
 >             if not hasattr(self, '_sync_lock'):
 >                 try:
 >                     self._sync_lock = threading.RLock()
 >                 finally:
 >                     global_lock.release()  # release first
 >             self._sync_lock.acquire()
 >         try:
 >             meth(self, *args, **kwargs)
 >         finally:
 >             self._sync_lock.release()
 >     return inner

Unnecessarily holding a lock while acquiring another can be a nasty
source of deadlock or at least delay.  Another source of problems is
holding a lock because an exception skipped past the release code.

Imagine in your original code:
 > A        try:
 > B            self._sync_lock.acquire()
 > C        except AttributeError:
 > D            global_lock.acquire()
 > E            if not hasattr(self, '_sync_lock'):
 > F                self._sync_lock = threading.RLock()
 > G            self._sync_lock.acquire()
 > H            global_lock.release()
 > I        meth(self, *args, **kwargs)
 > J        self._sync_lock.release()

Thread one executes:
     o = TheClass()
     v = o.themethod()  # and executes: A, B, C, D, E, F
         # Now thread1 holds only global_lock
Then Thread two executes:
     w = o.themethod()  # and executes: A, B, I
         # now thread 2 holds o._sync_lock

Thread one cannot proceed (it needs o._sync_lock) until Thread two
completes its code.  If, for example, the method body in Thread two
calls an unrelated synchronized method (perhaps on another object)
and must create another _sync_lock, Threads one and two will be
deadlocked.


--Scott David Daniels
scott.daniels at acm.org