From ef509e48b17ddd2c73e3030bb0d54dd672b4ae44 Mon Sep 17 00:00:00 2001 From: "cyrille@nnamrak.org" Date: Fri, 20 Sep 2013 13:19:08 +0000 Subject: [PATCH] Added SafeRename to better handle problems with git processes locking directories. This solves a problem with "os.rename" sometimes failing with an exception after cloning a dependency to a temporary directory. It's possible the dying git processes still hold a handle to the directory. Since gclient delete the temporary directory regardless of the success of the process, it results in a lot of GB downloaded for nothing. SafeRename solves this by retrying a few times if the renaming fails, sleeping one second every time to get other processes the time to release their lock on the directory. It gives up retrying after 15 times. BUG= R=maruel@chromium.org Review URL: https://chromiumcodereview.appspot.com/23875041 git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@224372 0039d316-1c4b-4281-b951-d872f2087c98 --- gclient_scm.py | 4 ++-- gclient_utils.py | 23 +++++++++++++++++++++++ tests/gclient_utils_test.py | 4 ++-- 3 files changed, 27 insertions(+), 4 deletions(-) diff --git a/gclient_scm.py b/gclient_scm.py index bb719ebf7..acb31ef15 100644 --- a/gclient_scm.py +++ b/gclient_scm.py @@ -873,8 +873,8 @@ class GitWrapper(SCMWrapper): print(str(e)) print('Retrying...') gclient_utils.safe_makedirs(self.checkout_path) - os.rename(os.path.join(tmp_dir, '.git'), - os.path.join(self.checkout_path, '.git')) + gclient_utils.safe_rename(os.path.join(tmp_dir, '.git'), + os.path.join(self.checkout_path, '.git')) finally: if os.listdir(tmp_dir): print('\n_____ removing non-empty tmp dir %s' % tmp_dir) diff --git a/gclient_utils.py b/gclient_utils.py index 8385deb05..e3c504a9d 100644 --- a/gclient_utils.py +++ b/gclient_utils.py @@ -96,6 +96,29 @@ def FileWrite(filename, content, mode='w'): f.write(content) +def safe_rename(old, new): + """Renames a file reliably. + + Sometimes os.rename does not work because a dying git process keeps a handle + on it for a few seconds. An exception is then thrown, which make the program + give up what it was doing and remove what was deleted. + The only solution is to catch the exception and try again until it works. + """ + # roughly 10s + retries = 100 + for i in range(retries): + try: + os.rename(old, new) + break + except OSError: + if i == (retries - 1): + # Give up. + raise + # retry + logging.debug("Renaming failed from %s to %s. Retrying ..." % (old, new)) + time.sleep(0.1) + + def rmtree(path): """shutil.rmtree() on steroids. diff --git a/tests/gclient_utils_test.py b/tests/gclient_utils_test.py index c42a01dc6..b1d3eb214 100755 --- a/tests/gclient_utils_test.py +++ b/tests/gclient_utils_test.py @@ -38,8 +38,8 @@ class GclientUtilsUnittest(GclientUtilBase): 'GCLIENT_CHILDREN_LOCK', 'GClientChildren', 'SplitUrlRevision', 'SyntaxErrorToError', 'UpgradeToHttps', 'Wrapper', 'WorkItem', 'codecs', 'lockedmethod', 'logging', 'os', 'pipes', 'Queue', - 're', 'rmtree', 'safe_makedirs', 'stat', 'subprocess', 'subprocess2', - 'sys', 'tempfile', 'threading', 'time', 'urlparse', + 're', 'rmtree', 'safe_makedirs', 'safe_rename', 'stat', 'subprocess', + 'subprocess2', 'sys', 'tempfile', 'threading', 'time', 'urlparse', ] # If this test fails, you should add the relevant test. self.compareMembers(gclient_utils, members)