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

Status: RESOLVED WONTFIX

(Firefox Build System :: General, defect)

Description

[Joey Armstrong [:joey]]

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.

Comment 1

[Joey Armstrong [:joey]]

Attachment: directory deps needed to avoid a race condition

Comment 2

[Joey Armstrong [: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.

Comment 3

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

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 4

[Mike Hommey [:glandium]]

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.

Comment 5

[Joey Armstrong [:joey]]

(In reply to Kyle Huey [:khuey] ([e-mail]) [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.

Comment 6

[Mike Hommey [:glandium]]

(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

Comment 7

[Joey Armstrong [:joey]]

(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.

Comment 8

[Mike Hommey [:glandium]]

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