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

RESOLVED FIXED in Firefox 51

Status

()

defect
--
critical
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: gkw, Assigned: jandem)

Tracking

(Blocks 1 bug, {assertion, jsbugmon, testcase})

Trunk
mozilla52
x86_64
macOS
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox51 fixed, firefox52 fixed)

Details

(Whiteboard: [jsbugmon:update])

Attachments

(2 attachments)

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.
=== 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
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)
Blocks: 956899
Flags: needinfo?(terrence.d.cole)
Flags: needinfo?(jdemooij)
Not s-s, shell-only and safe crash.
Group: javascript-core-security
Posted patch PatchSplinter Review
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.
Assignee: nobody → jdemooij
Status: NEW → ASSIGNED
Flags: needinfo?(terrence.d.cole)
Flags: needinfo?(jdemooij)
Attachment #8799356 - Flags: review?(nfroyd)
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.
Attachment #8799356 - Flags: review?(nfroyd) → review+
Pushed by jandemooij@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8efe890d2a28
Remove js::Thread's infallible constructor. r=froydnj
https://hg.mozilla.org/mozilla-central/rev/8efe890d2a28
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Are we able to easily backport this issue to mozilla-aurora? If so, that would be nice.
Flags: needinfo?(jdemooij)
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.
Flags: needinfo?(jdemooij)
Attachment #8799356 - Flags: approval-mozilla-aurora?
Comment on attachment 8799356 [details] [diff] [review]
Patch

Fix a fuzzing assertion error. Take it in 51 aurora.
Attachment #8799356 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.