1570157 - Hit MOZ_CRASH(Expected available JSContext) at js/src/vm/HelperThreads.cpp:1232 with evalInWorker

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

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

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

for (let i = 0; i < 99; i++) {
    evalInWorker("");
}

Backtrace:

#0  js::GlobalHelperThreadState::getFirstUnusedContext (this=<optimized out>, locked=...) at js/src/vm/HelperThreads.cpp:1232
#1  0x000055b3b581c882 in js::AutoSetHelperThreadContext::AutoSetHelperThreadContext (this=0x7fca2d9fe7d8) at js/src/vm/HelperThreads.cpp:475
#2  0x000055b3b58222ab in js::GCParallelTask::runFromHelperThread (this=0x7fca2af77f80, lock=...) at js/src/vm/HelperThreads.cpp:1773
#3  0x000055b3b58228a0 in js::HelperThread::handleGCParallelWorkload (this=0x7fca2da44800, lock=...) at js/src/vm/HelperThreads.cpp:1798
/snip

For detailed crash information, see attachment.

This has blown up jsfunfuzz, so setting [fuzzblocker].

Comment 1

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

Attachment: Detailed Crash Information

Comment 2

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

Due to skipped revisions, the first bad revision could be any of:
changeset: https://hg.mozilla.org/mozilla-central/rev/a56b3fe80336
user: Kristen Wright
date: Mon Jul 29 19:08:45 2019 +0000
summary: Bug 1559659 - 2. Create, initialize, destroy vector of JSContext* with GlobalHelperThreadState r=jandem

changeset: https://hg.mozilla.org/mozilla-central/rev/0eff8ea5dbf1
user: Kristen Wright
date: Mon Jul 29 19:09:00 2019 +0000
summary: Bug 1559659 - 3. Get rid of per-thread jscontext instantiation and use the global pool instead r=jandem

Kris, is bug 1559659 a likely regressor?

Comment 3

[Kris Wright :KrisWright]

(In reply to Gary Kwong [:gkw] [:nth10sd] from comment #2)

Due to skipped revisions, the first bad revision could be any of:
changeset: https://hg.mozilla.org/mozilla-central/rev/a56b3fe80336
user: Kristen Wright
date: Mon Jul 29 19:08:45 2019 +0000
summary: Bug 1559659 - 2. Create, initialize, destroy vector of JSContext* with GlobalHelperThreadState r=jandem

changeset: https://hg.mozilla.org/mozilla-central/rev/0eff8ea5dbf1
user: Kristen Wright
date: Mon Jul 29 19:09:00 2019 +0000
summary: Bug 1559659 - 3. Get rid of per-thread jscontext instantiation and use the global pool instead r=jandem

Kris, is bug 1559659 a likely regressor?

There should always be as many JSContexts as helper threads (meaning there should always be a context available for every helper thread to take) but in this case, it looks like there were 9 helper threads and only the default number of JSContexts in the list. From my understanding SetFakeCPUCount was called with --cpu-count=9 after js::CreateHelperThreadsState which means there were not enough JSContexts to serve the 9 threads.
I think the best solution would be to just initialize the global context list after calling SetFakeCPUCount, so that the list initializes before JS_NewContext initializes the main-thread context but after any changes could be made to the CPU count. I'll have a patch up shortly with these changes.

Comment 4

[Kris Wright :KrisWright]

Attachment: Bug 1570157 - Ensure there are enough JSContexts after calling SetFakeCPUCount. r=jandem

calling SetFakeCpuCount may cause GlobalHelperThreadState to create more threads than there are JSContexts in the list. This adds a function to check the length of the context list against the new thread count, and if there are not enough JSContexts, append more to the list.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c38db894e4f9
Ensure there are enough JSContexts after calling SetFakeCPUCount. r=jandem

Comment 6

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/c38db894e4f9