1991250 - Crash in [@ mozilla::LinkedListElement<T>::setPreviousUnsafe] with dom.script_loader.navigation_cache =TRUE (STR in comment 6)

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Mayank Bansal]

Crash report: https://crash-stats.mozilla.org/report/index/10c2ee8a-0313-4b52-be52-8a31e0250927

MOZ_CRASH Reason:

MOZ_RELEASE_ASSERT(!listElem->isInList())

Top 10 frames:

0  xul.dll  mozilla::LinkedListElement<JS::loader::ScriptLoadRequest>::setPreviousUnsafe(...  mfbt/LinkedList.h:334
0  xul.dll  mozilla::LinkedList<JS::loader::ScriptLoadRequest>::insertBack(JS::loader::Sc...  mfbt/LinkedList.h:526
0  xul.dll  JS::loader::ScriptLoadRequestList::AppendElement(JS::loader::ScriptLoadRequest*)  js/loader/ScriptLoadRequestList.cpp:42
0  xul.dll  mozilla::dom::ScriptLoader::CalculateCacheFlag(JS::loader::ScriptLoadRequest*)  dom/script/ScriptLoader.cpp:2875
1  xul.dll  mozilla::dom::ModuleLoader::CompileJavaScriptModule(JSContext*, JS::CompileOp...  dom/script/ModuleLoader.cpp:236
1  xul.dll  mozilla::dom::ModuleLoader::CompileFetchedModule(JSContext*, JS::Handle<JSObj...  dom/script/ModuleLoader.cpp:223
2  xul.dll  JS::loader::ModuleLoaderBase::CreateModuleScript(JS::loader::ModuleLoadRequest*)  js/loader/ModuleLoaderBase.cpp:977
2  xul.dll  JS::loader::ModuleLoaderBase::OnFetchComplete(JS::loader::ModuleLoadRequest*,...  js/loader/ModuleLoaderBase.cpp:748
3  xul.dll  JS::loader::ModuleLoadRequest::OnFetchComplete(nsresult)  js/loader/ModuleLoadRequest.h:105
3  xul.dll  mozilla::dom::ScriptLoader::PrepareLoadedRequest(JS::loader::ScriptLoadReques...  dom/script/ScriptLoader.cpp:4689

Getting this when opening https://www.tomshardware.com/.

Repros with all addons disabled. Does not repro on a fresh profile.

Edit: I dont repro now. The crash had something to do with dom.script_loader.navigation_cache = TRUE

Comment 1

[Mayank Bansal]

I have tried to get a Windbg Time travel debug trace, hopefully its useful. Will DM that to dev if needed.

Comment 2

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

The Bugbug bot thinks this bug should belong to the 'Core::JavaScript Engine' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 3

[Mayank Bansal]

Attachment: about:support

Comment 4

[Mayank Bansal]

Bug 1717778 added these asserts.

Comment 5

[Mayank Bansal]

Attachment: demonstration.mp4

Comment 6

[Mayank Bansal]

STR:

1. Create fresh profile.
2. Set dom.script_loader.navigation_cache = TRUE, restart the browser
3. Go to https://www.tomshardware.com/ and let the page load
4. Now RAPIDLY, CTRL+click (or middle-mouse click) on the newsarticle link to the right 15-20 times to open the link in new background tabs.
5. Atleast one of the newly opened tabs will crash.

Comment 7

[Tooru Fujisawa [:arai]]

Thank you for reporting.
It's nice to have a testcase, but also at the same time it's surprising that there are several reports.

It sounds like we really should rename the pref, to make it more explicit that it's not yet ready.

Comment 8

[Tooru Fujisawa [:arai]]

Setting to S4 given this is specific to hidden pref.

Comment 9

[Tooru Fujisawa [:arai]]

I see the pref listed in FeatureManifest.yaml, does it mean we're doing some remote experiment?
If that's the case, this bug is more critical, given that it means the user doesn't go through the about:config warnings.
Otherwise it remains S4.

Also, I'm going to rename the pref in bug 1931908, to make it more explicit that it's testing-only and not yet ready for daily usage.
(At least I saw some mention of the pref in some forums)
Let me know if renaming it breaks something in the experiment side.

Comment 10

[Denis Palmeiro [:denispal]]

(In reply to Tooru Fujisawa [:arai] from comment #9)

I see the pref listed in FeatureManifest.yaml, does it mean we're doing some remote experiment?
If that's the case, this bug is more critical, given that it means the user doesn't go through the about:config warnings.
Otherwise it remains S4.

It was a potential candidate for some performance experimentation Tyler is doing, but I mentioned to him to double check with you before he includes it in any experiment. At the moment it is not included in any experiments. For reference, his first experiment is here: https://experimenter.services.mozilla.com/nimbus/adaptive-performance-baseline-pref-optimization/summary/.

Comment 11

[Tooru Fujisawa [:arai]]

Thank you for the details!
Then I'll keep this S4 and address bug 1931908 at the same time.

Comment 12

[Tooru Fujisawa [:arai]]

Attachment: Bug 1991250 - Do not add to the mCacheableDependencyModules twice. r?nbp!

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/77e682ad81f5
https://hg.mozilla.org/integration/autoland/rev/076b1c7a5a67
Do not add to the mCacheableDependencyModules twice. r=nbp

Comment 14

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/076b1c7a5a67

Comment 15

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

Set release status flags based on info from the regressing bug 1980152