1811635 - shutdown window leak through ModuleLoaderBase::mFetchingModules

Status: NEW

(Core :: JavaScript Engine, task, P3)

Description

[Andrew McCreight [:mccr8]]

mtigley encountered a shutdown leak while working on bug 1805233. It is intermittent, but rather frequent.

Here's her description of how to reproduce it in a debug build:

In about:logins.html add another script tag that imports a module importing "lit.all.js": https://searchfox.org/mozilla-central/rev/eb00c7365e72ac422acd97f6eee3c26926b786cd/browser/components/aboutlogins/content/aboutLogins.html#29 .
In my case, I just duplicate the script tag for moz-button-group import.
Run a test that open about:logins.
eg: ./mach mochitest browser/components/protections/test/browser/browser_protections_lockwise.js

I was able to reproduce it by applying the two patches from her try push and then running the entire directory of that test. (I had a bit of trouble reproducing it with DMD logging so maybe running the entire directory helped, or maybe I just got lucky.)

Anyways, I then applied my heap scan mode technique to get CC and DMD heap scan logs, giving me the contents of heap objects.

Here's why the inner window of about:logins is being held alive:

0x111dee4c0 ScriptLoadRequest::mLoader -->
0x111ddbc90 ModuleLoaderBase::mGlobalObject -->
0x1130bc800 nsGlobalWindowInner about:logins

The ScriptLoadRequest at 0x111dee4c0 has a refcount of 3. The known edges are the mImports[i] field of the ScriptLoadRequest at 0x113e94c40, and the mRequest field of the LoadContextBase at 0x113e2d7a0.

Skipping an intermediate step or two, the cycle looks like it is:
0x111dee4c0 ScriptLoadRequest mLoader -->
0x111ddbc90 ModuleLoaderBase mFetchingModules -->
0x113e2c200 GenericNonExclusivePromise::Private -->
0x113e14cf0 Some Then promise thing from ModuleLoaderBase::StartOrRestartModuleLoad? -->
0x111dee4c0 back to the original ScriptLoadRequest

I'm not sure exactly how the promise ownership chains work, but I'm guessing the basic issue is that mFetchingModules is not being cleared as expected, and the MozPromise can't be cycle collected because it is some fancy threadsafe thing.

mtigley was able to work around the leak: "it looks like moving the script tag to the template container seemed to "fix" the leak: https://hg.mozilla.org/mozilla-central/rev/e863493f5734 ."

I don't know anything about this script loading and I didn't actually look at the test case but hopefully that's useful in understanding what is going on here.

Comment 1

[Jon Coppeard (:jonco)]

Thanks for the detailed analysis.

Comment 2

[Mike Conley (:mconley) (:⚙️)]

We think we've found a workaround for bug 1839844, but this feels like something that will bite us again and again. mgaudet, any chance somebody on the JS team could help us solve this at a lower level?

Comment 3

[Matthew Gaudet (he/him) [:mgaudet]]

Redirecting the needinfo to Jon who will have more insight into the family of issues than I.

Comment 4

[Jon Coppeard (:jonco)]

0x111ddbc90 ModuleLoaderBase mFetchingModules -->

This edge should be removed when the fetch completes. I guess it's possible for a fetch to never complete though.

0x113e2c200 GenericNonExclusivePromise::Private -->

It's annoying that MozPromise doesn't play well with the cycle collector. Perhaps removing use of this would fix the problem.