1776832 - Leak when system ESM is used

Status: RESOLVED FIXED

(Core :: XPConnect, defect)

Description

[Tooru Fujisawa [:arai]]

Attachment: test.patch.txt

Apply the attachment, and run test browser/components/pagedata/tests/browser/browser_pagedata_basic.js
test passes, but leak is detected.

 0:13.40 GECKO(54974) WARNING: YOU ARE LEAKING THE WORLD (at least one JSRuntime and everything alive inside it, that is) AT JS_ShutDown TIME.  FIX THIS!
 0:13.40 GECKO(54974) [Parent 54974, Main Thread] ###!!! ASSERTION: 29 dynamic atom(s) with non-zero refcount: downloads-item-font-size-factor,protections-popup-trackersView,key_hideThisAppCmdMac,private-browsing-indicator,PanelUI-bookmarks,translate-notification-icon,manBookmarkKb,downloadsCmd_clearList,menu_printSetup,key_preferencesCmdMac,context-openlinkprivate,find-button,key_responsiveDesignMode,pageStyleMenu,extension-new-tab,downloadHoveringButton,desktop,key_inspector,Unmute,chromeclass-toolbar,...: 'nonZeroRefcountAtomsCount == 0', file /Users/arai/projects/mozilla-unified/xpcom/ds/nsAtomTable.cpp:443

Comment 1

[Tooru Fujisawa [:arai]]

possibly related failure
https://treeherder.mozilla.org/jobs?repo=try&group_state=expanded&revision=cd4cb9a330e83d5f008175e578bfbfaa286f1c76&selectedTaskRun=ObM-1uAIRoGVMCA2GbKWZQ.0

[task 2022-07-01T10:11:28.019Z] 10:11:28     INFO - GECKO(11722) | Hit MOZ_CRASH(mozilla::LinkedList<mozilla::dom::ContentParent>::~LinkedList() [T = mozilla::dom::ContentParent] has a buggy user: it should have removed all this list's elements before the list's destruction) at /builds/worker/workspace/obj-build/dist/include/mozilla/LinkedList.h:466
[task 2022-07-01T10:11:28.020Z] 10:11:28     INFO - GECKO(11722) | #01: mozilla::LinkedList<mozilla::dom::ContentParent>::~LinkedList() [mfbt/LinkedList.h:462]
[task 2022-07-01T10:11:28.020Z] 10:11:28     INFO - GECKO(11722) | #02: mozilla::UniquePtr<mozilla::LinkedList<mozilla::dom::ContentParent>, mozilla::DefaultDelete<mozilla::LinkedList<mozilla::dom::ContentParent> > >::reset(mozilla::LinkedList<mozilla::dom::ContentParent>*) [mfbt/UniquePtr.h:305]
[task 2022-07-01T10:11:28.022Z] 10:11:28     INFO - GECKO(11722) | #03: ??? [/lib/x86_64-linux-gnu/libc.so.6 + 0x430f1]
[task 2022-07-01T10:11:28.023Z] 10:11:28     INFO - GECKO(11722) | #04: ??? [/lib/x86_64-linux-gnu/libc.so.6 + 0x431ea]
[task 2022-07-01T10:11:28.023Z] 10:11:28     INFO - GECKO(11722) | #05: __libc_start_main [/lib/x86_64-linux-gnu/libc.so.6 + 0x21b9e]
[task 2022-07-01T10:11:28.023Z] 10:11:28     INFO - GECKO(11722) | #06: ??? [/builds/worker/workspace/build/application/firefox/firefox + 0x44169]
[task 2022-07-01T10:11:28.024Z] 10:11:28     INFO - GECKO(11722) | #07: ??? (???:???)

Comment 2

[Andrew McCreight [:mccr8]]

Yes, that's another assertion that shows up when you leak.

Comment 3

[Tooru Fujisawa [:arai]]

This code in JSM case looks related

https://searchfox.org/mozilla-central/rev/7ac19055172e92d89675c3e221bfb0070e3aef0b/js/xpconnect/loader/mozJSModuleLoader.h#154-160

if (obj) {
  if (JS_HasExtensibleLexicalEnvironment(obj)) {
    JS::RootedObject lexicalEnv(mozilla::dom::RootingCx(),
                                JS_ExtensibleLexicalEnvironment(obj));
    JS_SetAllNonReservedSlotsToUndefined(lexicalEnv);
  }
  JS_SetAllNonReservedSlotsToUndefined(obj);

the equivalent might be necessary for ESM

Comment 4

[Tooru Fujisawa [:arai]]

Attachment: 0001-testing-patch-Bug-1776832-Clear-module-environment-o.patch

So far, clearing the module environment during mozJSModuleLoader::Unload seems to solve the leak in the testcase.

Comment 5

[Jon Coppeard (:jonco)]

(In reply to Tooru Fujisawa [:arai] from comment #4)
I feel that this shouldn't be necessary, but it's the equivalent of what mozJSModuleLoader already does for JSMs so let's take this to make ESM module loading work.

Comment 6

[Jon Coppeard (:jonco)]

Attachment: Bug 1776832 - Clear chrome module environments in shutdown r?arai

Comment 7

[Cristian Tuns]

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