Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(532)

Issue 8359018: Fix a concurrency issue happening with deep dependencies when the intermediary (Closed)

Created:
9 years, 2 months ago by M-A Ruel
Modified:
9 years, 2 months ago
CC:
chromium-reviews, Dirk Pranke, M-A Ruel
Visibility:
Public.

Description

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 Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=106636

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+37 lines, -3 lines) Patch
M gclient_scm.py View 4 chunks +4 lines, -2 lines 0 comments Download
M gclient_utils.py View 1 chunk +20 lines, -0 lines 0 comments Download
M tests/gclient_scm_test.py View 3 chunks +12 lines, -0 lines 0 comments Download
M tests/gclient_utils_test.py View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 4 (0 generated)
M-A Ruel
It occurs only on fresh checkouts, checking in since it's seriously affecting our TPMs.
9 years, 2 months ago (2011-10-20 23:38:23 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/maruel@chromium.org/8359018/1
9 years, 2 months ago (2011-10-20 23:38:33 UTC) #2
commit-bot: I haz the power
Change committed as 106636
9 years, 2 months ago (2011-10-20 23:44:23 UTC) #3
Dirk Pranke
9 years, 2 months ago (2011-10-21 00:00:45 UTC) #4
lgtm

Powered by Google App Engine
This is Rietveld 408576698