Closed Bug 474041 Opened 15 years ago Closed 15 years ago

[FIX]Leak with two elements whose ID matches an <svg:use xlink:href>


(Core :: SVG, defect, P3)






(Reporter: jruderman, Assigned: bzbarsky)



(Keywords: fixed1.9.1, memory-leak, testcase)


(3 files)

Attached image testcase
Steps to reproduce:
1. Run Firefox debug with trace-refcnt enabled (e.g. XPCOM_MEM_LEAK_LOG=2).
2. Load the testcase.
3. Quit Firefox.

Result: trace-refcnt reports that a bunch of stuff leaked, including nsGenericElements, for a total of 221907 bytes of instrumented objects.
Attached file trace-refcnt leak log
Attached patch FixSplinter Review
No tests included yet because I haven't figured out the best way to test this reliably.

What I see happen is that we do CreateAnonymousContent on the <use> twice in this testcase.  The first time presumably during initial load, the second time when we move it before the first rect.  I'm not sure why we don't do it the second time we move the <use>; I need to dig some more into that.

In any case, the second time we're in CreateAnonymousContent we call LookupHref(), which calls nsReferencedElement::Reset, which calls Unlink().  There is an mPendingNotification triggered by the same DOM mutation that moved the <use>, so we Clear() it but do NOT drop it (without this patch).  Now we go ahead and set up our new anonymous content.  Then when the <use> is moved again, we end up calling nsReferencedElement::Observe for the change in which content maps to the given id.  Now mPendingNotification is non-null, so we give it a reference to the new content (in this case the outer rect).  But it has a null mOwner, so it'll never drop the ref to this content (and in fact it might have Run() already; since mOwner is null it can't tell the owner to drop it).  So now the nsReferencedElement holds a ref to the outer rect, the outer rect holds a ref to the <use>, and we have a reference cycle.  Since mPendingNotification doesn't participate in cycle collection, we never collect the cycle.
Assignee: nobody → bzbarsky
Attachment #357466 - Flags: superreview?(roc)
Attachment #357466 - Flags: review?(roc)
Oh, and we might want to take this in 1.9.1.
Summary: Leak with two elements whose ID matches an <svg:use xlink:href> → [FIX]Leak with two elements whose ID matches an <svg:use xlink:href>
Attachment #357466 - Flags: superreview?(roc)
Attachment #357466 - Flags: superreview+
Attachment #357466 - Flags: review?(roc)
Attachment #357466 - Flags: review+
Flags: blocking1.9.1+
Priority: -- → P3
> I'm not sure why we don't do it the second time we move the <use>

Because <rect> gets a leaf frame, so no frame construction for the <use> at that point.
Pushed and
Closed: 15 years ago
Flags: in-testsuite+
Keywords: fixed1.9.1
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.2a1
You need to log in before you can comment on or make changes to this bug.