Initial step into making Dependency thread safe

gclient_utils.py

 # Copyright (c) 2011 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 """Generic utils."""
 
 import errno
 import logging
 import os
 import Queue
@@ skipping 438 matching lines @@
   if not config_path:
     print "Can't find %s" % config_file
     return None
 
   env = {}
   execfile(config_path, env)
   config_dir = os.path.dirname(config_path)
   return config_dir, env['entries']
 
 
+def lockedmethod(method):
+  """Method decorator that holds self.lock for the duration of the call."""
+  def inner(self, *args, **kwargs):
+    try:
+      try:
+        self.lock.acquire()
+      except KeyboardInterrupt:
+        print >> sys.stderr, 'Was deadlocked'
+        raise
+      return method(self, *args, **kwargs)
+    finally:
+      self.lock.release()
+  return inner
+
+
 class WorkItem(object):
   """One work item."""
-  def __init__(self):
+  def __init__(self, name):
     # A list of string, each being a WorkItem name.
-    self.requirements = []
+    self._requirements = set()
     # A unique string representing this work item.
-    self.name = None
+    self._name = name
+    self.lock = threading.RLock()

[Dirk Pranke, 2011/09/14 20:00:39]

Why RLock instead of Lock()?

[M-A Ruel, 2011/09/14 20:03:03]

I'll need a RLock due to some locking recursion when accessing the properties.
It's maybe possible to get away with a Lock() but that will be slightly more
complex in some cases.

 
+  @lockedmethod
   def run(self, work_queue):
     """work_queue is passed as keyword argument so it should be
     the last parameters of the function when you override it."""
     pass
 
+  @property
+  def name(self):
+    return self._name
+
+  @property
+  @lockedmethod
+  def requirements(self):

[Dirk Pranke, 2011/09/14 20:00:39]

Do you really need a lock here? Isn't this guaranteed to be safe because of the
GIL? (granted it probably doesn't hurt to have it locked).

[M-A Ruel, 2011/09/14 20:03:03]

More or less but I want to reduce the GIL assumption in the code since the
current state of Dependency class is quite racy.

+    return tuple(self._requirements)
+
 
 class ExecutionQueue(object):
   """Runs a set of WorkItem that have interdependencies and were WorkItem are
   added as they are processed.
 
   In gclient's case, Dependencies sometime needs to be run out of order due to
   From() keyword. This class manages that all the required dependencies are run
   before running each one.
 
   Methods of this class are thread safe.
@@ skipping 153 matching lines @@
         logging.info('Caught exception in thread %s' % self.item.name)
         logging.info(str(sys.exc_info()))
         work_queue.exceptions.put(sys.exc_info())
       logging.info('Task %s done' % self.item.name)
 
       work_queue.ready_cond.acquire()
       try:
         work_queue.ready_cond.notifyAll()
       finally:
         work_queue.ready_cond.release()