Closed Bug 393974 Opened 13 years ago Closed 13 years ago

Tree walkers leak with a non-null filter


(Core :: DOM: Core & HTML, defect)

Not set





(Reporter: Waldo, Assigned: Waldo)


(Keywords: memory-leak, regression)


(2 files, 1 obsolete file)

Attached file Testcase
This needs cycle collector macrology.

We need to get this in one of our testing harnesses in such a way that the test will fail if we leak, ideally by hooking up mochitests to leak-gauge and failing whenever anything leaks.  This bug was *fixed* once, but the cycle collector landing didn't include cycle collection in nsTreeWalker.  This is *trivially* catchable with leak-gauge; we should ensure it never happens again.
Flags: in-testsuite?
Flags: blocking1.9?
I filed bug 393993 on getting support in Mochitest to fail on window/document/docshell leaks.  In the meantime, hacking in the environment variables to shows that without this patch, we leak a bunch of stuff related to the included test (running only dom/tests/mochitest/bugs), whereas with it, we only leak about:blank (to be dealt with in the nuclear holocaust I intend to wage upon test leaks, so that we can make this test actually fail without the patch).
Attachment #278583 - Flags: superreview?(jonas)
Attachment #278583 - Flags: review?(jonas)
Attachment #278583 - Flags: review?(jonas) → review+
Comment on attachment 278583 [details] [diff] [review]
Patch, with mochitest that would leak without it

sr=me, but I think there are macros that implement cycle collecting addref/release/unlink/traverse using a single macro, for simple unlink/traverses like this.
Attachment #278583 - Flags: superreview?(jonas) → superreview+
NS_IMPL_CYCLE_COLLECTION_3 in nsCycleCollectionParticipant.h
Flags: blocking1.9? → blocking1.9+
That macro wasn't working for me when I originally tried it, but a closer look at the error message showed the fairly obvious mistake.  This patch, to be committed momentarily, uses the _3 macro.
Attachment #278583 - Attachment is obsolete: true
Attachment #278677 - Flags: superreview+
Attachment #278677 - Flags: review+
Checked in on trunk, with a test, but since the test wouldn't fail if this were regressed I'm leaving the flag as-is; I'll flip it when bug 393993 is fixed.
Closed: 13 years ago
Resolution: --- → FIXED
Component: DOM: Core → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.