From 6c48a304d4701b0c513eb26db2a8cadd01f18b4b Mon Sep 17 00:00:00 2001 From: "maruel@chromium.org" Date: Thu, 20 Oct 2011 23:44:20 +0000 Subject: [PATCH] Fix a concurrency issue happening with deep dependencies when the intermediary directory doesn't exist, on fresh checkout and --jobs >1. It happens in the case where there is 3 dependencies: a a/b/c/d a/b/c/e 'a' is processed first. Then 'a/b/c/d' and 'a/b/c/e' are fired simultaneously. If both are not present, both svn checkout tries to mkdir b and b/c and a race condition occurs between their call for isdir() and mkdir(). This works around the issue by creating the intermediary directories ourself before calling out svn and managing the error ourself. TBR=dpranke@chromium.org BUG= TEST=fresh checkout shouldn't be flaky Review URL: http://codereview.chromium.org/8359018 git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@106636 0039d316-1c4b-4281-b951-d872f2087c98 --- gclient_scm.py | 6 ++++-- gclient_utils.py | 20 ++++++++++++++++++++ tests/gclient_scm_test.py | 12 ++++++++++++ tests/gclient_utils_test.py | 2 +- 4 files changed, 37 insertions(+), 3 deletions(-) diff --git a/gclient_scm.py b/gclient_scm.py index 8519fa9d0..546aa08cd 100644 --- a/gclient_scm.py +++ b/gclient_scm.py @@ -213,6 +213,7 @@ class GitWrapper(SCMWrapper): rev_type = "hash" if not os.path.exists(self.checkout_path): + gclient_utils.safe_makedirs(os.path.dirname(self.checkout_path)) self._Clone(revision, url, options) files = self._Capture(['ls-files']).splitlines() file_list.extend([os.path.join(self.checkout_path, f) for f in files]) @@ -508,7 +509,7 @@ class GitWrapper(SCMWrapper): # create it, so we need to do it manually. parent_dir = os.path.dirname(self.checkout_path) if not os.path.exists(parent_dir): - os.makedirs(parent_dir) + gclient_utils.safe_makedirs(parent_dir) percent_re = re.compile('.* ([0-9]{1,2})% .*') def _GitFilter(line): @@ -775,6 +776,7 @@ class SVNWrapper(SCMWrapper): rev_str = '' if not os.path.exists(self.checkout_path): + gclient_utils.safe_makedirs(os.path.dirname(self.checkout_path)) # We need to checkout. command = ['checkout', url, self.checkout_path] command = self._AddAdditionalUpdateFlags(command, options, revision) @@ -906,7 +908,7 @@ class SVNWrapper(SCMWrapper): # information is not stored next to the file, so we will have to # re-export the file every time we sync. if not os.path.exists(self.checkout_path): - os.makedirs(self.checkout_path) + gclient_utils.safe_makedirs(self.checkout_path) command = ["export", os.path.join(self.url, filename), os.path.join(self.checkout_path, filename)] command = self._AddAdditionalUpdateFlags(command, options, diff --git a/gclient_utils.py b/gclient_utils.py index 918b489c7..e65893468 100644 --- a/gclient_utils.py +++ b/gclient_utils.py @@ -171,6 +171,26 @@ def rmtree(path): RemoveDirectory = rmtree +def safe_makedirs(tree): + """Creates the directory in a safe manner. + + Because multiple threads can create these directories concurently, trap the + exception and pass on. + """ + count = 0 + while not os.path.exists(tree): + count += 1 + try: + os.makedirs(tree) + except OSError, e: + # 17 POSIX, 183 Windows + if e.errno not in (17, 183): + raise + if count > 40: + # Give up. + raise + + def CheckCallAndFilterAndHeader(args, always=False, **kwargs): """Adds 'header' support to CheckCallAndFilter. diff --git a/tests/gclient_scm_test.py b/tests/gclient_scm_test.py index c69ad2fca..44d23aa30 100755 --- a/tests/gclient_scm_test.py +++ b/tests/gclient_scm_test.py @@ -142,6 +142,10 @@ class SVNWrapperTestCase(BaseTestCase): gclient_scm.os.path.exists(join(self.base_path, '.hg')).AndReturn(False) # Checkout. gclient_scm.os.path.exists(self.base_path).AndReturn(False) + parent = gclient_scm.os.path.dirname(self.base_path) + gclient_scm.os.path.exists(parent).AndReturn(False) + gclient_scm.os.makedirs(parent) + gclient_scm.os.path.exists(parent).AndReturn(True) files_list = self.mox.CreateMockAnything() gclient_scm.scm.SVN.RunAndGetFileList( options.verbose, @@ -167,6 +171,10 @@ class SVNWrapperTestCase(BaseTestCase): gclient_scm.os.path.exists(join(self.base_path, '.git')).AndReturn(False) gclient_scm.os.path.exists(join(self.base_path, '.hg')).AndReturn(False) gclient_scm.os.path.exists(self.base_path).AndReturn(False) + parent = gclient_scm.os.path.dirname(self.base_path) + gclient_scm.os.path.exists(parent).AndReturn(False) + gclient_scm.os.makedirs(parent) + gclient_scm.os.path.exists(parent).AndReturn(True) files_list = self.mox.CreateMockAnything() gclient_scm.scm.SVN.Capture(['--version']).AndReturn('svn, version 1.6') gclient_scm.scm.SVN.RunAndGetFileList( @@ -255,6 +263,10 @@ class SVNWrapperTestCase(BaseTestCase): gclient_scm.os.path.exists(join(self.base_path, '.hg')).AndReturn(False) # Checkout. gclient_scm.os.path.exists(self.base_path).AndReturn(False) + parent = gclient_scm.os.path.dirname(self.base_path) + gclient_scm.os.path.exists(parent).AndReturn(False) + gclient_scm.os.makedirs(parent) + gclient_scm.os.path.exists(parent).AndReturn(True) files_list = self.mox.CreateMockAnything() gclient_scm.scm.SVN.Capture(['--version'] ).AndReturn('svn, version 1.5.1 (r32289)') diff --git a/tests/gclient_utils_test.py b/tests/gclient_utils_test.py index 444cbc510..f124d3bec 100755 --- a/tests/gclient_utils_test.py +++ b/tests/gclient_utils_test.py @@ -36,7 +36,7 @@ class GclientUtilsUnittest(GclientUtilBase): 'PrintableObject', 'RemoveDirectory', 'SoftClone', 'SplitUrlRevision', 'SyntaxErrorToError', 'WorkItem', 'errno', 'lockedmethod', 'logging', 'os', 'Queue', 're', 'rmtree', - 'stat', 'subprocess2', 'sys','threading', 'time', + 'safe_makedirs', 'stat', 'subprocess2', 'sys','threading', 'time', ] # If this test fails, you should add the relevant test. self.compareMembers(gclient_utils, members)