1499525 - Leaks of WeakMaps and Unboxed Layouts in non-debug build

Status: RESOLVED WONTFIX

(Core :: JavaScript: GC, enhancement, P5)

Description

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

By chance I happened to enable the LinkedList assertion (https://searchfox.org/mozilla-central/rev/3d989d097fa35afe19e814c6d5bc2f2bf10867e1/mfbt/LinkedList.h#461 ) on a non-debug build. On some xpcshell tests I was getting completely reproducible assertions on WeakMaps and Unboxed Layouts.

After investigation I traced them back to these code blocks:
https://searchfox.org/mozilla-central/rev/3d989d097fa35afe19e814c6d5bc2f2bf10867e1/js/src/gc/Zone.cpp#95
https://searchfox.org/mozilla-central/rev/28fe656de6a022d1fc01bbd3811023fca99cf223/js/src/vm/Realm.cpp#79,82

Where we simply added code that would clear the linked lists if we were in debug mode to avoid the assertion firing.

It seems like this could be actively leaking memory on Release; and that the fix is simply to always clear the linked list in the destructor (and not just do it in Debug mode.)  But I know very very little about this code so that could be all wrong...

Comment 1

[Jon Coppeard (:jonco)]

(In reply to Tom Ritter [:tjr] from comment #0)
> Where we simply added code that would clear the linked lists if we were in
> debug mode to avoid the assertion firing.

That's not exactly what's happening here.  Sometimes the browser leaks JS GC things at shutdown and this used to trigger a ton of assertions in the JS engine.  This is a pain because it makes it look like a problem in the JS engine and stops the leak actually getting reported.  So we added code to skip these asserts in this case (that's the runtime_->gc.shutdownCollectedEverything() check).

I was working on better reporting of these leaks in bug 1407593 but they seemed to go away for a while.  Now there are back again though.

Comment 2

[Andrew McCreight [:mccr8]]

Leaking memory in non-debug builds is expected behavior.

Comment 3

[Andrew McCreight [:mccr8]]

The only reason we don't leak in debug and ASan builds is that we do a bunch of cleanup work at shutdown.