Commit 6ca8bf80 authored by maruel@chromium.org's avatar maruel@chromium.org

Retry "Initial step into making Dependency thread safe""

I had forgot one access to _requirements. Now that tests are run, this change is
safer to commit.

R=dpranke@chromium.org
BUG=
TEST=


Review URL: http://codereview.chromium.org/7909012

git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@101856 0039d316-1c4b-4281-b951-d872f2087c98
parent 8a312c56
...@@ -145,9 +145,8 @@ class Dependency(GClientKeywords, gclient_utils.WorkItem): ...@@ -145,9 +145,8 @@ class Dependency(GClientKeywords, gclient_utils.WorkItem):
# self.dependencies and self.requirements are read and modified from # self.dependencies and self.requirements are read and modified from
# multiple threads at the same time. Sad. # multiple threads at the same time. Sad.
GClientKeywords.__init__(self) GClientKeywords.__init__(self)
gclient_utils.WorkItem.__init__(self) gclient_utils.WorkItem.__init__(self, name)
self.parent = parent self.parent = parent
self.name = name
self.url = url self.url = url
self.parsed_url = None self.parsed_url = None
# These 2 are only set in .gclient and not in DEPS files. # These 2 are only set in .gclient and not in DEPS files.
...@@ -169,8 +168,6 @@ class Dependency(GClientKeywords, gclient_utils.WorkItem): ...@@ -169,8 +168,6 @@ class Dependency(GClientKeywords, gclient_utils.WorkItem):
self.processed = False self.processed = False
# This dependency had its hook run # This dependency had its hook run
self.hooks_ran = False self.hooks_ran = False
# Required dependencies to run before running this one:
self.requirements = set()
# Post process the url to remove trailing slashes. # Post process the url to remove trailing slashes.
if isinstance(self.url, basestring): if isinstance(self.url, basestring):
...@@ -201,7 +198,7 @@ class Dependency(GClientKeywords, gclient_utils.WorkItem): ...@@ -201,7 +198,7 @@ class Dependency(GClientKeywords, gclient_utils.WorkItem):
# self.parent is implicitly a requirement. This will be recursive by # self.parent is implicitly a requirement. This will be recursive by
# definition. # definition.
if self.parent and self.parent.name: if self.parent and self.parent.name:
self.requirements.add(self.parent.name) self._requirements.add(self.parent.name)
# For a tree with at least 2 levels*, the leaf node needs to depend # For a tree with at least 2 levels*, the leaf node needs to depend
# on the level higher up in an orderly way. # on the level higher up in an orderly way.
...@@ -219,10 +216,10 @@ class Dependency(GClientKeywords, gclient_utils.WorkItem): ...@@ -219,10 +216,10 @@ class Dependency(GClientKeywords, gclient_utils.WorkItem):
for i in range(0, root_deps.index(self.parent)): for i in range(0, root_deps.index(self.parent)):
value = root_deps[i] value = root_deps[i]
if value.name: if value.name:
self.requirements.add(value.name) self._requirements.add(value.name)
if isinstance(self.url, self.FromImpl): if isinstance(self.url, self.FromImpl):
self.requirements.add(self.url.module_name) self._requirements.add(self.url.module_name)
if self.name and self.should_process: if self.name and self.should_process:
def yield_full_tree(root): def yield_full_tree(root):
...@@ -238,10 +235,16 @@ class Dependency(GClientKeywords, gclient_utils.WorkItem): ...@@ -238,10 +235,16 @@ class Dependency(GClientKeywords, gclient_utils.WorkItem):
continue continue
# Step 1: Find any requirements self may need. # Step 1: Find any requirements self may need.
if self.name.startswith(posixpath.join(obj.name, '')): if self.name.startswith(posixpath.join(obj.name, '')):
self.requirements.add(obj.name) self._requirements.add(obj.name)
# Step 2: Find any requirements self may impose. # Step 2: Find any requirements self may impose.
if obj.name.startswith(posixpath.join(self.name, '')): if obj.name.startswith(posixpath.join(self.name, '')):
obj.requirements.add(self.name) try:
# Access to a protected member _requirements of a client class
# pylint: disable=W0212
obj.lock.acquire()
obj._requirements.add(self.name)
finally:
obj.lock.release()
def LateOverride(self, url): def LateOverride(self, url):
"""Resolves the parsed url from url. """Resolves the parsed url from url.
......
...@@ -456,19 +456,45 @@ def GetGClientRootAndEntries(path=None): ...@@ -456,19 +456,45 @@ def GetGClientRootAndEntries(path=None):
return config_dir, env['entries'] 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): class WorkItem(object):
"""One work item.""" """One work item."""
def __init__(self): def __init__(self, name):
# A list of string, each being a WorkItem name. # A list of string, each being a WorkItem name.
self.requirements = [] self._requirements = set()
# A unique string representing this work item. # A unique string representing this work item.
self.name = None self._name = name
self.lock = threading.RLock()
@lockedmethod
def run(self, work_queue): def run(self, work_queue):
"""work_queue is passed as keyword argument so it should be """work_queue is passed as keyword argument so it should be
the last parameters of the function when you override it.""" the last parameters of the function when you override it."""
pass pass
@property
def name(self):
return self._name
@property
@lockedmethod
def requirements(self):
return tuple(self._requirements)
class ExecutionQueue(object): class ExecutionQueue(object):
"""Runs a set of WorkItem that have interdependencies and were WorkItem are """Runs a set of WorkItem that have interdependencies and were WorkItem are
......
...@@ -35,7 +35,7 @@ class GclientUtilsUnittest(GclientUtilBase): ...@@ -35,7 +35,7 @@ class GclientUtilsUnittest(GclientUtilBase):
'MakeFileAutoFlush', 'MakeFileAnnotated', 'PathDifference', 'MakeFileAutoFlush', 'MakeFileAnnotated', 'PathDifference',
'PrintableObject', 'RemoveDirectory', 'SoftClone', 'SplitUrlRevision', 'PrintableObject', 'RemoveDirectory', 'SoftClone', 'SplitUrlRevision',
'SyntaxErrorToError', 'WorkItem', 'SyntaxErrorToError', 'WorkItem',
'errno', 'logging', 'os', 'Queue', 're', 'rmtree', 'errno', 'lockedmethod', 'logging', 'os', 'Queue', 're', 'rmtree',
'stat', 'subprocess2', 'sys','threading', 'time', 'stat', 'subprocess2', 'sys','threading', 'time',
] ]
# If this test fails, you should add the relevant test. # If this test fails, you should add the relevant test.
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment