Closed
Bug 463411
Opened 16 years ago
Closed 15 years ago
nsinstall can race creating directories
Categories
(Firefox Build System :: General, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla1.9.3a1
People
(Reporter: ted, Assigned: ted)
References
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
timeless
:
review+
|
Details | Diff | Splinter Review |
/local/bin/make -C public export
make[5]: Entering directory `/d/build/obj-firefox-opt-libxul/content/canvas'
/local/bin/make -C public export
make[6]: Entering directory `/d/build/obj-firefox-opt-libxul/content/canvas/publ
ic'make[6]: Entering directory `/d/build/obj-firefox-opt-libxul/content/base/pub
lic'
/c/mozilla-build/moztools/bin/nsinstall -D ../../../dist/include/content/c/mozil
la-build/moztools/bin/nsinstall -D ../../../dist/include/content
/c/mozilla-build/moztools/bin/nsinstall -D ../../../dist/sdk/include
..\..\..\dist\include\content: Could not create the directory: File exists
make[6]: *** [../../../dist/include/content] Error 3
So the PARALLEL_DIRS patch and the followup patch from bug 462440 exposed a race condition in "nsinstall -D". You can see it in the code here:
http://hg.mozilla.org/mozilla-central/annotate/874aba8a9a8a/config/nsinstall.c#l351
and here (the version apparently shipped with MozillaBuild):
http://bonsai.mozilla.org/cvsblame.cgi?file=buildtools%2Fwindows%2Fsource%2Fshmsdos%2Fnsinstall.c&rev=&cvsroot=%2Fcvsroot&mark=157-159#124
Basically, those mkdir checks should check that errno != EEXIST, since if two "nsinstall -D" processes are spawned simultaneously, they can both pass the "directory does not exist" check, and then both run mkdir, with one succeeding, and one failing with EEXIST.
we're running in a loop which means we want to use chdir instead of checking EEXIST. if chdir works, then we can continue through our loop.
if we checked EEXIST, we'd still have to perform the chdir, which makes it a useless check :)
Assignee | ||
Comment 2•15 years ago
|
||
Simple enough. Can't get this to fail with a little test shellscript. (nsinstall -D /foo & nsinstall -D /foo, in a loop)
Assignee: nobody → ted.mielczarek
Status: NEW → ASSIGNED
Attachment #409131 -
Flags: review?(timeless)
Attachment #409131 -
Flags: review?(timeless) → review+
Assignee | ||
Comment 3•15 years ago
|
||
You can hit this without -D, since nsinstall.exe uses the same codepath to create destination directories for files.
Also, my previous patch is insufficient to fix this in all cases. New patch in a minute.
Summary: nsinstall can race when called with -D → nsinstall can race creating directories
Assignee | ||
Comment 4•15 years ago
|
||
This fixes the problem I was seeing. Two nsinstalls were racing doing something like:
nsinstall <a bunch of files> ../../../_tests/testing/mochitest/tests/a/b/c
The race was actually creating 'a', I think, but with my previous patch it would still fail because it was skipping that _wchdir(path), which gets you back to the cwd, so the next mkdir would fail. This patch works reliably for me. (I was reliably failing in the same spot when building with -j12 on an 8-core machine.)
Attachment #409131 -
Attachment is obsolete: true
Attachment #409320 -
Flags: review?(timeless)
Attachment #409320 -
Flags: review?(timeless) → review+
Assignee | ||
Comment 5•15 years ago
|
||
Pushed to m-c:
http://hg.mozilla.org/mozilla-central/rev/e857761718d5
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a1
Updated•7 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•