1885237 - Tweak memory leak test helper to figure out any leak happening in tests

Status: NEW

(DevTools :: Framework, enhancement)

Description

[Alexandre Poirot [:ochameau]]

Bug 1727837 introduced some test helper to be able to investigate why a given object is being leaked.
It will print the possible retaining path of one particular object.
Unfortunately, this API is currently designed to work easily in "allocation tests" in: devtools/client/framework/test/allocations/.
These tests are using startRecordingAllocations which is only exposed in that specific test folder.

May be these test helpers should be moved to shared-head so that any DevTools test can use it?
These test helpers are document over here.
Also, may be a simpler test helper should be exposed in order to only focused on "tracked objects" and not on displaying all possible leaks?

Comment 1

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

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

Comment 2

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1885237 - [devtools] Experiment trying to figure out a JS leak using existing DevTools memory helpers.

This is a case study for a leak being figured out thanks to these helpers.

This all started with a docshell leak reported by tests.
Running $ ./mach mochitest --headless devtools/client/styleeditor/test/browser_styleeditor_private_perwindowpb.js
on a debug build would report:

devtools/client/styleeditor/test/browser_styleeditor_private_perwindowpb.js
  FAIL devtools/client/styleeditor/test/browser_styleeditor_private_perwindowpb.js - leaked 1 window(s) until shutdown [url = chrome://browser/content/browser.xhtml]
  FAIL devtools/client/styleeditor/test/browser_styleeditor_private_perwindowpb.js - leaked 1 window(s) until shutdown [url = about:blank]

It took me a little while, but by reading my patch I ended up understanding that it was related to the WatcherActor class.
Its _browserElement attribute was ultimately leaking the browser.xhtml docshell.
Nullifying in its destroy method would get rid of the mochitest docshell leak report.

Then... understanding why the WatcherActor instance is leaked is much more challenging as actors are references in many possible ways between each others.

So I first used tracked-objects.sys.mjs in order to tell to DevTools memory test helper about this particular object
I would like to investigate track(this); in WatcherActor.destroy method.
Then at the end of the test, once everything should have been freed, I request info about the leak:

// First force freeing everything
Cu.forceGC();
Cu.forceCC();

// Wait a bit
await wait(2000);

// Retrive all tracked objects, if they are still in memory
// Freed object won't be returned by getAllNodeIds.
const TrackedObjects = ChromeUtils.importESModule(
 "resource://devtools/shared/test-helpers/tracked-objects.sys.mjs"
);
const objectNodeIds = TrackedObjects.getAllNodeIds();
if (objectNodeIds.length) {
  // Request the memory tracker to displat the path from a global retaining the leaked objects
  tracker.traceObjects(objectNodeIds);
}

Now tracker has be to defined. Ideally before running your test so that the tracker can record
all JS objects allocation site. This appears to be a crucial information here to figure out the leak.

// We need all that magick because spidermonkey API can only debug the shared global from a distinct global/compartment
// That what we do here. Load allocation-tracker.js from a distinct global.
// (this will be significantly simplier once we move that code to ESMs)
const {
  useDistinctSystemPrincipalLoader,
  releaseDistinctSystemPrincipalLoader,
} = ChromeUtils.importESModule(
  "resource://devtools/shared/loader/DistinctSystemPrincipalLoader.sys.mjs"
);
const requester = {};
const loader = useDistinctSystemPrincipalLoader(requester);

const { allocationTracker } = loader.require(
  "chrome://mochitests/content/browser/devtools/shared/test-helpers/allocation-tracker.js"
);

// We watch all globals, not only the shared global as devtools involves many other globals with its loaders.
const tracker = allocationTracker({ watchAllGlobals: true });

And with all that we get the following output:

 # Tracing: Object@resource://devtools/shared/protocol/Pool.js:27
 0:10.51 GECKO(75430) ### Path(s) from root:
 0:11.00 GECKO(75430) - other@no-stack:undefined.mJSObj
 0:11.00 GECKO(75430)  \--> Object@resource://devtools/server/actors/resources/index.js:338.watcherActor
 0:11.00 GECKO(75430)  \--> Object@resource://devtools/shared/protocol/Pool.js:27
 0:11.00 GECKO(75430) - other@no-stack:undefined.mJSObj
 0:11.00 GECKO(75430)  \--> Object@resource://devtools/server/actors/resources/index.js:338.onNetworkEventAvailable
 0:11.00 GECKO(75430)  \--> BoundFunctionObject@resource://devtools/server/actors/resources/index.js:341.**UNKNOWN SLOT 2**
 0:11.00 GECKO(75430)  \--> Object@resource://devtools/shared/protocol/Pool.js:27
 0:11.00 GECKO(75430) - other@no-stack:undefined.mJSObj
 0:11.00 GECKO(75430)  \--> Object@resource://devtools/server/actors/resources/index.js:338.onNetworkEventUpdated
 0:11.00 GECKO(75430)  \--> BoundFunctionObject@resource://devtools/server/actors/resources/index.js:345.**UNKNOWN SLOT 2**
 0:11.00 GECKO(75430)  \--> Object@resource://devtools/shared/protocol/Pool.js:27
 0:11.00 GECKO(75430) - other@no-stack:undefined.mJSObj
 0:11.00 GECKO(75430)  \--> Object@resource://devtools/server/actors/resources/index.js:338.watcherActor
 0:11.00 GECKO(75430)  \--> Object@resource://devtools/shared/protocol/Pool.js:27._targetConfigurationListActor
 0:11.00 GECKO(75430)  \--> Object@resource://devtools/shared/protocol/Pool.js:27.watcherActor
 0:11.00 GECKO(75430)  \--> Object@resource://devtools/shared/protocol/Pool.js:27
 0:11.00 GECKO(75430) - other@no-stack:undefined.mJSObj
 0:11.00 GECKO(75430)  \--> Object@resource://devtools/server/actors/resources/index.js:338.watcherActor
 0:11.01 GECKO(75430)  \--> Object@resource://devtools/shared/protocol/Pool.js:27._threadConfigurationListActor
 0:11.01 GECKO(75430)  \--> Object@resource://devtools/shared/protocol/Pool.js:27.watcherActor
 0:11.01 GECKO(75430)  \--> Object@resource://devtools/shared/protocol/Pool.js:27

(I added a tweak in this changeset to get the full URLs for the allocation side to understand what index.js was reference to)
(also tweaked the number of path being displayed, the shortest, the first one isn't helpful at all)

This isn't super obvious but index.js:338 is where we instantiate all the Resource Watchers.
The one leaking the Watcher Actor is NetworkEventWatcher:
https://searchfox.org/mozilla-central/rev/ff6e63804e6f7b548a26338708663093ce16be11/devtools/server/actors/resources/network-events.js#43
We instantiate a NetworkEventWatcher on index.js:338. That's what the helpers says.
This object has an watcherActor attribute refering to the leaked object.
The leaked object is at the end of the path, it is the one allocated at Pool.js:27:
https://searchfox.org/mozilla-central/rev/ff6e63804e6f7b548a26338708663093ce16be11/devtools/shared/protocol/Pool.js#25-27
It looks like all Actors, which ultimately inherit from this Pool class have their memory allocation site set at that particular position...
Also the output can probably be improved it isn't super clear what is what between allocated objects and edges.

What would have been extra helpful here would have been to have shown the JS Class constructor name instead of "Object" everywhere!

I don't know JS API so well but it seems to be fetched from here:
https://searchfox.org/mozilla-central/rev/ff6e63804e6f7b548a26338708663093ce16be11/js/src/vm/UbiNode.cpp#344
May be that can be tweaked to lookup for a constructor name in case of Object?