Detect chrome windows that outlive their owning mochitest and report retention paths
Categories
(Testing :: Mochitest, enhancement)
Tracking
(Not tracked)
People
(Reporter: florian, Assigned: florian)
References
Details
Attachments
(3 files)
The shutdown leak check only sees windows that are still alive at shutdown. It misses windows that stay alive across several tests and are then freed when a later test does something that incidentally drops references to them, or when early shutdown cleanup releases those references. Those leaks are real bugs (they grow CC times, consume memory, and can OOM CI) but are currently invisible.
Add a between-tests detector that flags any chrome window which is still alive several GC/CC cycles after its owning test has torn down the docshell, and labels the failure with the owning test path.
Two pieces:
-
DOMWindowTrackerintesting/mochitest/browser-test.jswatchesdebug-domwindow-created/debug-domwindow-destroyedplusinner-window-destroyedto track each chrome window's serial and docshell state. It also countsgarbage-collector-endandcycle-collector-endnotifications. A window whose docshell has been torn down but is still alive after several natural GC/CC cycles past the end of its owning test is reported as a between-tests leak, attributed to that test. -
testing/mochitest/ShutdownLeakPathFinder.sys.mjsaugments those reports with CC-graph-based retention paths so the failure surfaces actionable information about what is keeping the window alive.
The detector counts natural cycles rather than forcing GC/CC on every test boundary, so the per-test cost is negligible; detection may be delayed across one or more subsequent tests but only the genuinely persistent leaks make it that far.
|
Assignee | |
Comment 1•4 months ago
|
||
Adds a between-tests leak check to the browser-chrome harness so we
fail a test that keeps a window alive past its scope, instead of
waiting for the shutdown leak check (which often misses these
because the extra GC/CCs around shutdown collect the leak first).
Mechanism:
-
'debug-domwindow-created' now includes the inner window's id in
its observer data, so the JS tracker can map the windowID
delivered by 'inner-window-destroyed' (a uint64 wrapped in
nsISupportsPRUint64) back to a tracker entry. -
DOMWindowTracker observes 'inner-window-destroyed',
'garbage-collector-end' and 'cycle-collector-end'. When
inner-window-destroyed fires it flips a per-window 'detached'
flag; Tester.nextTest calls a new DOMWindowTracker.noteTestEnded
right after setting currentTest to null at testEnd, which
snapshots the current GC/CC counters on every detached entry
whose owning test just ended. Windows whose
inner-window-destroyed runnable arrives after the owning test
had already ended snapshot endedAt themselves, in the observer. -
Between tests (from Tester.nextTest's waitForFocus callback,
before execTest is called for the next test),
runDetachedLeakCheckBetweenTests logs a test_status FAIL for
every detached window that has survived POST_TEST_LEAK_THRESHOLD
(3) natural GC and CC end events past its owning test's end and
is still in liveWindows. The FAIL is stamped with the window's
creation timestamp so resourcemonitor's overlap check lands it
inside the owning test's marker, and the blame is attributed to
the test that owned the window rather than to the test that
happened to be running when the threshold was crossed. -
resourcemonitor.test_status recognizes the new
'WindowLeakAfterDocShellDestroyed' subtest the same way it
already recognizes the end-of-tests 'Shutdown' subtest, so the
blamed test's marker is flipped from PASS to FAIL.
The check intentionally does not invoke the shutdown leak path
finder here: the path finder's synchronous Cu.forceCC tends to
collect the very leak we're trying to characterize. Retention-path
tracing for these between-tests leaks is added separately.
Implicitly DEBUG-only: 'debug-domwindow-created' only fires in
DEBUG builds, so non-DEBUG runs never populate innerIDToSerial and
the detection silently does nothing.
|
|
Assignee | |
Comment 2•4 months ago
|
||
Replace runDetachedLeakCheckBetweenTests's inline test_status FAIL
reporting with a ShutdownLeakPathFinder call, so each between-tests
window leak is reported with a retention path (when one exists)
instead of just a serial/address line.
ShutdownLeakPathFinder gains options that let it serve both the
existing end-of-tests shutdown leak check and the new between-tests
check:
-
'subtest' and 'messagePrefix' customize the test_status record
produced for each leaked window. 'messagePrefix' may also be a
function of windowInfo so callers can include per-window data in
the failure message. Defaults preserve the existing
'Shutdown' / 'leaked window until shutdown' labels. -
'failOnTransient' (default true, unchanged for the shutdown
check) controls whether windows not found in the CC graph at all,
or windows the CC marked as a garbage cycle, are reported as
test_status FAIL. The between-tests caller passes false: those
cases there are typically slow-to-collect rather than persistent,
and demoting them to a single info-level summary line keeps the
failure log focused on actual leaks. -
findAndPrintPaths now returns the set of addresses that actually
produced a FAIL. DOMWindowTracker uses it to only mark serials as
reported when they FAILed, so transient cases that ended up
persisting after all still get one more chance at the end-of-tests
shutdown leak check. For those transient cases the post-test-end
snapshot is advanced to "now" so they have to survive another
full threshold before being reconsidered. -
The reporting loop is wrapped in
deactivateBuffering()/activateBuffering() so the path lines and
test_status FAILs land in the live test log instead of only in
the end-of-run failure summary, and a one-line summary of how
many paths were reported / not in graph / no path to root is
printed when the loop ends.
Caveat: ShutdownLeakPathFinder triggers a synchronous Cu.forceCC to
build the CC graph. That synchronous CC can collect leaks that a
normal async CC (sliced, with JS running between slices to revive
weak references) would not, so applying this patch may reduce the
number of windows the between-tests check reports compared to the
inline-FAIL version. This is acceptable here: end-of-tests is too
late to characterize the retention paths of such leaks, and a
between-tests path with reduced sensitivity is still strictly
better information than no path at all.
|
|
Assignee | |
Comment 3•4 months ago
|
||
The retention-path tracer in ShutdownLeakPathFinder calls Cu.forceCC to
build the CC graph. That forced CC also unlinks all the white nodes it
finds, so leaked windows that the between-tests check is trying to
characterize can be collected mid-analysis: the leak is real, but the
graph the analysis ran against no longer contains the node we were
looking up, and the failure gets demoted to a "transient" line instead
of a FAIL with a retention path.
Add a |logOnly| attribute to nsICycleCollectorListener. When set, the
cycle collector runs ScanRoots as usual (so the listener still sees the
full set of nodes, edges, roots and garbage), then skips CollectWhite
entirely and advances straight to CleanupPhase. The graph state is
cleaned up but no objects are unlinked, so the next normal CC is the
one that actually frees any garbage observed during the log-only run.
Wire the new flag through ShutdownLeakPathFinder.findAndPrintPaths via
an options.logOnly knob, and pass it from
DOMWindowTracker.runDetachedLeakCheckBetweenTests so the between-tests
analysis no longer perturbs the heap it is reporting on. The existing
end-of-tests shutdown leak check is unchanged and continues to do a
normal (collecting) CC.
Description
•