1457475 - Crash in js::GlobalHelperThreadState::finishParseTask

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Marcia Knous [:marcia]]

This bug was filed from the Socorro interface and is
report bp-a38d9b33-58e6-47c9-83d1-6f6100180427.
=============================================================

These Mac and Windows crashes started showing up using 20180426220144: https://bit.ly/2Hu9BgZ. Not sure if they are expected due to Bug 1452114? ni on :jandem

Top 10 frames of crashing thread:

0 xul.dll js::GlobalHelperThreadState::finishParseTask js/src/vm/HelperThreads.cpp:1709
1 xul.dll mozilla::ScriptPreloader::MaybeFinishOffThreadDecode js/xpconnect/loader/ScriptPreloader.cpp:980
2 xul.dll mozilla::ScriptPreloader::WaitForCachedScript js/xpconnect/loader/ScriptPreloader.cpp:863
3 xul.dll mozilla::ScriptPreloader::GetCachedScript js/xpconnect/loader/ScriptPreloader.cpp:849
4 xul.dll mozJSComponentLoader::ObjectForLocation js/xpconnect/loader/mozJSComponentLoader.cpp:826
5 xul.dll mozJSComponentLoader::LoadModule js/xpconnect/loader/mozJSComponentLoader.cpp:442
6 xul.dll nsComponentManagerImpl::KnownModule::Load xpcom/components/nsComponentManager.cpp:717
7 xul.dll nsFactoryEntry::GetFactory xpcom/components/nsComponentManager.cpp:1748
8 xul.dll nsComponentManagerImpl::CreateInstanceByContractID xpcom/components/nsComponentManager.cpp:1046
9 xul.dll nsComponentManagerImpl::GetServiceByContractID xpcom/components/nsComponentManager.cpp:1409

=============================================================

Comment 1

[Jan de Mooij [:jandem]]

Attachment: Patch

Some of these look like OOMs but not all of them. Something is failing but not reporting an exception.

For now we can demote this MOZ_DIAGNOSTIC_ASSERT to MOZ_ASSERT, maybe fuzzing will find something.

Comment 2

[Jon Coppeard (:jonco)]

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

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

Good plan.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/45603dbcb3bb
Demote MOZ_DIAGNOSTIC_ASSERT to MOZ_ASSERT. r=jonco

Comment 4

[Jason Orendorff [:jorendorff]]

I'd like to know if the frontend or the worker thread machinery is to blame. Can we assert on the worker thread that this doesn't happen? (Maybe in HelperThread::handleParseWorkload.)

Comment 5

[Jason Orendorff [:jorendorff]]

<jandem> jorendorff: we could add a similar assert to the main thread frontend code
<jorendorff> yes, i was just thinking that
<jorendorff> jandem: seems like a very good idea if we really want fuzzer help

Comment 6

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/45603dbcb3bb

Comment 7

[Jan de Mooij [:jandem]]

Hm it looks like many of these are actually *decode* tasks instead of *parse* tasks, despite it being called finishParseTask, so we should probably add similar asserts to the XDR decoder.

Comment 8

[Jan de Mooij [:jandem]]

OK the XDR decoder only reports an exception if TranscodeResult_Throw is used, but there are other failure TranscodeResults that don't report a pending exception so the helper threads code should check for that. I'll file another followup.

Comment 9

[Jan de Mooij [:jandem]]

Filed bug 1458209 for the XDR bug.

Comment 10

[Calixte Denizet (:calixte)]

No more crashes since the patch landed.