1774111 - Crash in [@ mozilla::dom::AutoJSAPI::Init] from ModuleLoaderBase

Status: RESOLVED FIXED

(Core :: XPConnect, defect)

Description

[Andrew McCreight [:mccr8]]

Crash report: https://crash-stats.mozilla.org/report/index/1cc41e9c-f3db-416e-a525-eecfe0220613

Reason: SIGSEGV / SEGV_MAPERR

Top 10 frames of crashing thread:

0 libxul.so mozilla::dom::AutoJSAPI::Init dom/script/ScriptSettings.cpp:431
1 libxul.so JS::loader::ModuleLoaderBase::FinishDynamicImportAndReject js/loader/ModuleLoaderBase.cpp:793
2 libxul.so mozilla::dom::ScriptRequestProcessor::Run dom/script/ScriptLoader.cpp:490
3 libxul.so nsContentUtils::AddScriptRunner dom/base/nsContentUtils.cpp:5795
4 libxul.so mozilla::dom::ModuleLoader::OnModuleLoadComplete dom/script/ModuleLoader.cpp:129
5 libxul.so JS::loader::ModuleLoadRequest::LoadFinished js/loader/ModuleLoadRequest.cpp:193
6 libxul.so mozilla::MozPromise<bool, nsresult, false>::ThenValue<JS::loader::ModuleLoadRequest*, void  xpcom/threads/MozPromise.h:720
7 libxul.so mozilla::MozPromise<bool, nsresult, false>::ThenValueBase::ResolveOrRejectRunnable::Run xpcom/threads/MozPromise.h:487
8 libxul.so mozilla::RunnableTask::Run xpcom/threads/TaskController.cpp:467
9 libxul.so mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal xpcom/threads/TaskController.cpp:780

The recent failures look to be mostly on Fenix. Maybe it looks like ModuleLoaderBase is trying to call AutoJSAPI::Init with a null object?

Jon, any ideas? Maybe we're missing a null check somewhere?

Comment 1

[Hsin-Yi Tsai (she/her) (REO Fx 129) [:hsinyi]]

Hi jonco, a ping here to bring this (back) to your radar. Thank you.

Comment 2

[Jon Coppeard (:jonco)]

Likely the same issue as bug 1788532. I'll monitor this signature now a potential fix has landed in that bug.

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

Since the crash volume is low (less than 5 per week), the severity is downgraded to S3. Feel free to change it back if you think the bug is still critical.

For more information, please visit auto_nag documentation.

Comment 4

[Jon Coppeard (:jonco)]

There are no crashes in the last month with this signature on release 105 or later. I think that means that this is fixed, but please reopen if this recurs.

Comment 5

[Jon Coppeard (:jonco)]

I can still see this crash under xpc::NativeGlobal so reopening.

Comment 6

[Jon Coppeard (:jonco)]

Attachment: Bug 1774111 - Initialize AutoJAPI from the native global in the module loader r?yulia

We're getting a bunch of crashes related to initializing AutoJSAPI from a
JSObject pointer that turns out to be null. That overload of Init() gets the
native global from the JSObject and since we already have a native global in
the module loader we can use the overload that takes that instead. This
overload does a null check so we will catch the case where the global is null
(although that should also not happen).

This might just move crashes elsewhere but it's a reasonable tidyup.

Comment 7

[Jon Coppeard (:jonco)]

Attachment: Bug 1774111 - Check for already-completed request in ModuleLoaderBase::FinishDynamicImport r?yulia

I've looked at this for a while and still don't know how this can happen but it
does seem reasonable to add a check here that we haven't already completed the
request.

Depends on D162386

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/209561e1ca25
Initialize AutoJAPI from the native global in the module loader r=yulia
https://hg.mozilla.org/integration/autoland/rev/42b829aab112
Check for already-completed request in ModuleLoaderBase::FinishDynamicImport r=yulia

Comment 9

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/209561e1ca25
https://hg.mozilla.org/mozilla-central/rev/42b829aab112

Comment 10

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:jonco, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox108 to wontfix.

For more information, please visit auto_nag documentation.

Comment 11

[Jon Coppeard (:jonco)]

Comment on attachment 9304073 [details]
Bug 1774111 - Initialize AutoJAPI from the native global in the module loader r?yulia

Beta/Release Uplift Approval Request

• User impact if declined: Hopefully this will fix some crashes.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This is a fairly simple change and has been baking on central for four days.
• String changes made/needed: None
• Is Android affected?: Yes

Comment 12

[Donal Meehan [:dmeehan]]

Comment on attachment 9304073 [details]
Bug 1774111 - Initialize AutoJAPI from the native global in the module loader r?yulia

Approved for 108.0b6.

Comment 13

[Donal Meehan [:dmeehan]]

Comment on attachment 9304074 [details]
Bug 1774111 - Check for already-completed request in ModuleLoaderBase::FinishDynamicImport r?yulia

Approved for 108.0b6.

Comment 14

[Donal Meehan [:dmeehan]]

https://hg.mozilla.org/releases/mozilla-beta/rev/4b46ffa226b9
https://hg.mozilla.org/releases/mozilla-beta/rev/9493a11386bc

Comment 15

[Ryan VanderMeulen [:RyanVM]]

Please nominate these for ESR102 approval when you get a chance. They graft cleanly.

Comment 16

[Jon Coppeard (:jonco)]

Comment on attachment 9304073 [details]
Bug 1774111 - Initialize AutoJAPI from the native global in the module loader r?yulia

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Hopefully this fixes some crashes.
• User impact if declined: More crashes.
• Fix Landed on Version: 109
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This is a simple change and has been present in nightly for 11 days without problem.

Comment 17

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9304073 [details]
Bug 1774111 - Initialize AutoJAPI from the native global in the module loader r?yulia

Approved for 102.6esr.

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr102/rev/c4898a852b12
https://hg.mozilla.org/releases/mozilla-esr102/rev/f3e233eefb55

Comment 20

[BugBot [:suhaib / :marco/ :calixte]]

Copying crash signatures from duplicate bugs.