Closed Bug 463411 Opened 11 years ago Closed 10 years ago
nsinstall can race creating directories
/local/bin/make -C public export make: Entering directory `/d/build/obj-firefox-opt-libxul/content/canvas' /local/bin/make -C public export make: Entering directory `/d/build/obj-firefox-opt-libxul/content/canvas/publ ic'make: 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: *** [../../../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 :)
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)
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
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.)
Pushed to m-c: http://hg.mozilla.org/mozilla-central/rev/e857761718d5
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a1
You need to log in before you can comment on or make changes to this bug.