1308655 - Assertion failure: init(mozilla::Forward<F>(f), mozilla::Forward<Args>(args)...), at js/src/threading/Thread.h:110

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

The following testcase crashes on mozilla-central revision da986c9f1f72 (build with --enable-debug --enable-more-deterministic, run with --fuzzing-safe --ion-offthread-compile=off --ion-eager):

for (var j = 0; j < 2999; ++j) {
    evalInWorker("(function(){});timeout(1);");
}


Backtrace:

0   libsystem_kernel.dylib        	0x00007fff84d0d10a __semwait_signal + 10
1   libsystem_pthread.dylib       	0x00007fff887f7787 pthread_join + 444
2   js-dbg-64-dm-clang-darwin-da986c9f1f72	0x0000000108576de9 js::Thread::join() + 25 (Thread.cpp:117)
3   js-dbg-64-dm-clang-darwin-da986c9f1f72	0x00000001084b8606 main + 13334 (js.cpp:3321)
4   js-dbg-64-dm-clang-darwin-da986c9f1f72	0x00000001084b26b4 start + 52
/snip

For detailed crash information, see attachment.

Probably a threading issue, but GC is all over the stack so setting s-s as a start.

Comment 1

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Attachment: Detailed Crash Information

Comment 2

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

=== Treeherder Build Bisection Results by autoBisect ===

The "good" changeset has the timestamp "20160528055932" and the hash "17d8abb5b1803418ea206cd60fe5386dc8dded98".
The "bad" changeset has the timestamp "20160528071135" and the hash "132e391ed5e588241227dfb48083dcd57ac3142f".

Likely regression window: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=17d8abb5b1803418ea206cd60fe5386dc8dded98&tochange=132e391ed5e588241227dfb48083dcd57ac3142f

Comment 3

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/8aa6b27297b6
user:        Terrence Cole
date:        Wed May 25 09:41:57 2016 -0700
summary:     Bug 956899 - Use js::Thread for JS shell watchdog thread; r=jandem

Jan, is bug 956899 a likely regressor? (Also setting needinfo? from Terrence in case he has time for this)

Comment 4

[Jan de Mooij [:jandem]]

Not s-s, shell-only and safe crash.

Comment 5

[Jan de Mooij [:jandem]]

Attachment: Patch

js::Thread has a fallible init() method. It also has a constructor that calls init() and MOZ_RELEASE_ASSERTs it returns true. That assert is failing here for the watchdog thread.

This patch removes the constructor that calls init. It seems clearer to have each caller call init() explicitly and deal with OOM appropriately.

The only places I had to fix were the watchdog thread (this crash) and some jsapi-tests. The tests are slightly more verbose now, but it's test-only so I will take that over potential footguns.

Comment 6

[Nathan Froyd [:froydnj]]

Comment on attachment 8799356 [details] [diff] [review]
Patch

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

Should have realized that infallible constructors were a bad idea when I reviewed this code originally.  Thanks for cleaning this up.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8efe890d2a28
Remove js::Thread's infallible constructor. r=froydnj

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/8efe890d2a28

Comment 9

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Are we able to easily backport this issue to mozilla-aurora? If so, that would be nice.

Comment 10

[Jan de Mooij [:jandem]]

Comment on attachment 8799356 [details] [diff] [review]
Patch

Requesting approval to help fuzz testing. I think this is mostly NPOTB.

Approval Request Comment
[Feature/regressing bug #]: Older bug.
[User impact if declined]: Crashes when fuzzing.
[Describe test coverage new/current, TreeHerder]: Fixes tests, on m-c for a while.
[Risks and why]: Very low risk. It shouldn't affect the browser at all.
[String/UUID change made/needed]: None.

Comment 11

[Gerry Chang [:gchang]]

Comment on attachment 8799356 [details] [diff] [review]
Patch

Fix a fuzzing assertion error. Take it in 51 aurora.

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/179f83a4ec89