Closed Bug 755327 Opened 7 years ago Closed 7 years ago

modules/libmar/tests/Makefile.in - unit/ directory dep needed to avoid a race condition

Categories

(Firefox Build System :: General, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: joey, Assigned: joey)

Details

Attachments

(1 file)

Generation of $(TESTROOT)/unit/head_libmar.js can fail if the 2nd libs:: target has not been called to install/create the $(TESTROOT)/unit directory.  Define a new mkdir_deps dependency and use it as a pre-requisite for both libs:: targets.
Assignee: nobody → joey
Comment on attachment 624061 [details] [diff] [review]
directory deps needed to avoid a race condition

Added a mkdir_dep pre-requisite for the libs:: target to create the directory early.  A try job is on the way.
Attachment #624061 - Flags: review?(khuey)
Blocks: 742391
No longer blocks: 742391
Comment on attachment 624061 [details] [diff] [review]
directory deps needed to avoid a race condition

Review of attachment 624061 [details] [diff] [review]:
-----------------------------------------------------------------

I don't understand this.  How is the second libs target relevant?  Doesn't libs-xpcshell-tests (in config/makefiles/xpcshell.mk) create the directory?
Comment on attachment 624061 [details] [diff] [review]
directory deps needed to avoid a race condition

Review of attachment 624061 [details] [diff] [review]:
-----------------------------------------------------------------

cf. comment 3.
Attachment #624061 - Flags: review?(khuey) → review-
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → INVALID
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) [On vacation until June 4th] from comment #3)
> Comment on attachment 624061 [details] [diff] [review]
> directory deps needed to avoid a race condition
> 
> Review of attachment 624061 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I don't understand this.  How is the second libs target relevant?  Doesn't
> libs-xpcshell-tests (in config/makefiles/xpcshell.mk) create the directory?

If these targets are evaluated concurrently by make -j { on a *fast* desktop which I'm using }:

libs:: libs-xpcshell-tests
libs:: unit/head_libmar.js.in

The 2nd target can try and write into the $(TESTROOT) directroy before libs-xpcshell-tests has a chance to create it.  Deps are needed to avoid the race condition.
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
(In reply to Joey Armstrong [:joey] from comment #5)
> If these targets are evaluated concurrently by make -j { on a *fast* desktop
> which I'm using }:
> 
> libs:: libs-xpcshell-tests
> libs:: unit/head_libmar.js.in
> 
> The 2nd target can try and write into the $(TESTROOT) directroy before
> libs-xpcshell-tests has a chance to create it.  Deps are needed to avoid the
> race condition.

That doesn't seem possible:

$ cat > test.mk <<EOF
foo:
	sleep 1
	echo foo

libs:: foo

libs::
	echo bar
EOF
$ make -f /tmp/test.mk -j5 libs
sleep 1
echo foo
foo
echo bar
bar
Status: REOPENED → RESOLVED
Closed: 7 years ago7 years ago
Resolution: --- → INVALID
(In reply to Mike Hommey [:glandium] from comment #6)
> (In reply to Joey Armstrong [:joey] from comment #5)
> > If these targets are evaluated concurrently by make -j { on a *fast* desktop
> > which I'm using }:
> > 
> > libs:: libs-xpcshell-tests
> > libs:: unit/head_libmar.js.in
> > 
> > The 2nd target can try and write into the $(TESTROOT) directroy before
> > libs-xpcshell-tests has a chance to create it.  Deps are needed to avoid the
> > race condition.
> 
> That doesn't seem possible:
> 
> $ cat > test.mk <<EOF
> foo:
> 	sleep 1
> 	echo foo
> 
> libs:: foo
> 
> libs::
> 	echo bar
> EOF
> $ make -f /tmp/test.mk -j5 libs
> sleep 1
> echo foo
> foo
> echo bar
> bar

How fast is the machine you are on ?  I opened the bug because my desktop is able to produce this behavior.  If two libs targets are dependent and there is no mutex to gate entry a race condition will implicitly exist.

Behavior is transient and can be exposed by fast hardware.

If the 2nd lib target wins and attempts to write in the $(TESTROOT) directory { have to be fast enough to bypass the delay introduced by the command being run } redirect into $(TESTROOT) will fail.
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
I fail to see how this can happen, considering double-colon rules are executed in the order they appear in the file. And it doesn't matter how fast my machine is, the sleep 1 is there to make it clear that if it can do something else, it will (see below). And it doesn't.

$ cat > test.mk <<EOF
foo:
	sleep 1
	echo foo

bar::
	echo bar

libs:: foo bar
EOF

$ make -f /tmp/test.mk libs -j6
sleep 1
echo bar
bar
echo foo
foo
Status: REOPENED → RESOLVED
Closed: 7 years ago7 years ago
Resolution: --- → WONTFIX
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.