531135 - race condition with lock files in build-list.pl (components.list.lck: Permission denied, all-test-dirs.list.lck: Permission denied)

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[(not currently active) Ted Mielczarek]

I just hit two of these in a row, maybe I'm unlucky. I'm guessing this is something similar to bug 522804, but in mozLock.pm, which is what build-list.pl uses for locking. Maybe I should just rewrite build-list.pl in Python and use lockFile from utils.py (since I fixed the race conditions there already).

Comment 1

[Neil Lee [:neilio]]

Don't know why I'm cc'ed on this bug - did you mean Neil Deakin?

Comment 2

[(not currently active) Ted Mielczarek]

Wrong Neil. :)

Comment 3

[[e-mail]]

My original complaint to Ted referred to components.list.lck but for comparison, I just got this on SFU with a -j2 build:
>/usr/local/bin/perl -I/dev/fs/C/hg/comm/mozilla/config /dev/fs/C/hg/comm/mozilla/config/build-list.pl \
>  ../../../mozilla/_tests/xpcshell/all-test-dirs.list \
>  test_imap/unit
>/usr/local/bin/perl -I/dev/fs/C/hg/comm/mozilla/config /dev/fs/C/hg/comm/mozilla/config/build-list.pl \
>  ../../../mozilla/_tests/xpcshell/all-test-dirs.list \
>  test_addbook/unit
>/dev/fs/C/hg/mozilla/_tests/xpcshell/all-test-dirs.list.lck: Device busy
>gmake[6]: *** [libs] Error 16
>gmake[6]: Leaving directory `/dev/fs/C/hg/mailnews/addrbook/test'
>gmake[5]: *** [test_libs] Error 2
>gmake[5]: *** Waiting for unfinished jobs....
(Device busy is the SFU error when trying to delete a file that is open.
Text file busy is the SFU error when trying to delete a running executable.)

Comment 4

[(not currently active) Ted Mielczarek]

I rewrote build-list.pl in Python (it's only about 30 lines) since I already fixed the race condition bugs in the lockFile implementation in config/utils.py.

I'll have a patch up shortly.

Comment 5

[(not currently active) Ted Mielczarek]

Attachment: rewrite build-list.pl in Python

The implementation is pretty simple, I've included some basic unit tests as well. Most of this patch is just a search and replace from calls to build-list.pl to calls to buildlist.py. That bit in toolkit/xre/Makefile.in referencing embedding/browser/gtk/src/Makefile.in no longer seems to be relevant, so I've removed it.

Seemed easier to write this than to fumble through fixing mozLock.pm.

Comment 6

[Benjamin Smedberg]

Comment on attachment 415144 [details] [diff] [review]
rewrite build-list.pl in Python

Awesome. If you're going to remove build-list.pl right away, make sure comm-central guys know (or you can leave it around for a bit while they update).

Comment 7

[(not currently active) Ted Mielczarek]

Good call, CCing some more c-c folks. The removal is really just a search-and-replace, so the patch should be trivial.

Comment 8

[Phil Ringnalda (:philor)]

Attachment: c-c

That world with branches instead of ifdefs for other people's branches, where it would be a simple search/replace? I want to live there.

Comment 9

[[e-mail]]

ifdef MOZILLA_1_9_2_BRANCH
BUILDLIST = $(PERL) -I$(MOZILLA_DIR)/config $(MOZILLA_DIR)/config/build-list.pl
else
BUILDLIST = $(PYTHON) $(MOZILLA_DIR)/config/buildlist.py
endif

Comment 10

[(not currently active) Ted Mielczarek]

I can just leave build-list.pl there until you stop the madness, if that would make your life easier.

Comment 11

[(not currently active) Ted Mielczarek]

(but you have to promise to remove it for me later.)

Comment 12

[Phil Ringnalda (:philor)]

O, my future daughter! O, my future ducats!

Using BUILDLIST would be prettier now, but only if some unknown person in the unknowable future will realize that they should completely undo it while dropping 1_9_2 ifdefs, rather than just dropping the build-list.pl version and leaving our copy of rules.mk oddly different. My version looks like crap now, but ensures that no matter how blindly someone removes the ifdefs, we'll end up where we want to be, with as little difference from mozilla-central as possible. (Or, I guess we could define BUILDLIST in mozilla-central's copy, rather than having eight repeats of "$(PYTHON) $(MOZILLA_DIR)/config/buildlist.py".)

Comment 13

[(not currently active) Ted Mielczarek]

Pushed to m-c:
http://hg.mozilla.org/mozilla-central/rev/83b704c74e5f

Comment 14

[Mark Banner (:standard8)]

Pushed the c-c patch:
http://hg.mozilla.org/comm-central/rev/19de5e9ab3d2

Comment 15

[(not currently active) Ted Mielczarek]

I totally checked in tests with the Python rewrite.