1704561 - Crash in [@ JS::ThrowOnModuleEvaluationFailure]

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Gabriele Svelto [:gsvelto]]

Crash report: https://crash-stats.mozilla.org/report/index/add9db14-b8a8-4499-99f9-ed2310210412

Reason: EXCEPTION_ACCESS_VIOLATION_READ

Top 10 frames of crashing thread:

0 xul.dll JS::ThrowOnModuleEvaluationFailure js/src/vm/Modules.cpp:154
1 xul.dll mozilla::dom::ScriptLoader::EvaluateScript dom/script/ScriptLoader.cpp:3196
2 xul.dll mozilla::dom::ScriptLoader::ProcessRequest dom/script/ScriptLoader.cpp:2770
3 xul.dll mozilla::dom::ScriptLoader::ProcessPendingRequests dom/script/ScriptLoader.cpp:3591
4 xul.dll mozilla::dom::ScriptLoader::ProcessLoadedModuleTree dom/script/ScriptLoader.cpp:1357
5 xul.dll mozilla::dom::ModuleLoadRequest::LoadFinished dom/script/ModuleLoadRequest.cpp:206
6 xul.dll mozilla::dom::ScriptLoader::StartFetchingModuleDependencies dom/script/ScriptLoader.cpp:901
7 xul.dll mozilla::dom::ScriptLoader::ProcessFetchedModuleSource dom/script/ScriptLoader.cpp:605
8 xul.dll mozilla::dom::`anonymous namespace'::NotifyOffThreadScriptLoadCompletedRunnable::Run dom/script/ScriptLoader.cpp:2449
9 xul.dll mozilla::SchedulerGroup::Runnable::Run xpcom/threads/SchedulerGroup.cpp:143

This popped up in nightly recently (though there appear to be some older crashes). It seems to be happening on both macOS and Windows. I'm not sufficiently familiar with this code to tell what's going on.

Comment 1

[Tom S [:evilpie]]

The code in EvaluateScript looks a bit suspicious in general. For example here rv is set to NS_OK: https://searchfox.org/mozilla-central/rev/fa48ebee58d59fa846919c3e2c3122b08db57c9c/dom/script/ScriptLoader.cpp#3174. And checked again later: https://searchfox.org/mozilla-central/rev/fa48ebee58d59fa846919c3e2c3122b08db57c9c/dom/script/ScriptLoader.cpp#3184.

There is also no isObject() check for rval.

Comment 2

[Yulia Startsev [:yulia]]

The NS_OK looks possibly ok -- that should be caught by the promise rejection code, and shouldn't be handled here. Though, we should check for isObject.

There is a possibility that we don't have a resolution value.

Edit: I can reproduce a crash that looks a lot like this one if the rval is not initialized. I added a check to not set the evaluationPromise if we don't have an rval, the rest of the code already handles null pointer promises (and auto-fails). Hopefully this addresses the underlying cause.

Comment 3

[Yulia Startsev [:yulia]]

Attachment: Bug 1704561 - Ensure that Module return value is an object before binding it to evaluationPromise; r=evilpie!

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ced517f952ce
Ensure that Module return value is an object before binding it to evaluationPromise; r=evilpie

Comment 5

[Natalia Csoregi [:nataliaCs]]

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