Closed Bug 631421 Opened 9 years ago Closed 9 years ago

Leak nsDocument with two image-document iframes

Categories

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

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla2.0b12
Tracking Status
blocking2.0 --- final+
status1.9.2 --- .17-fixed

People

(Reporter: jruderman, Assigned: khuey)

References

(Blocks 1 open bug)

Details

(Keywords: memory-leak, regression, testcase, Whiteboard: [hardblocker])

Attachments

(2 files)

Attached file testcase
According to trace-refcnt, each time the testcase runs, 1 nsDocument leaks.

Related to bug 614149?
I guess the testcase needs to be changed a bit so that it points to some
valid image.
Yes. Changing it to a data URL makes the bug go away.
I can't reproduce this on Linux, even with the src being a valid image at a file:// address.
Still leaks for me on Mac (rev 5a4ab9b3fd2f).
Ok, I can reproduce this now on Linux.
OS: Mac OS X → All
Hardware: x86 → All
I think the bug here is that the check at http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsImageLoadingContent.cpp#454 fails and the observer ref is never released.

Nomming for blocking since this is a recent regression and looks like it would be easy for web content to hit.
Assignee: nobody → khuey
Blocks: 604262
Status: NEW → ASSIGNED
blocking2.0: --- → ?
Keywords: regression
The stack for the failing RemoveObserver call is

#0  nsImageLoadingContent::RemoveObserver (this=0x17601f8, aObserver=
    0x1ba9ce8)
    at /home/khuey/dev/mozilla-central3/content/base/src/nsImageLoadingContent.c
pp:454
#1  0x00007ffff4fcc8b7 in nsImageDocument::Destroy (this=0x1ba9710)
    at /home/khuey/dev/mozilla-central3/content/html/document/src/nsImageDocumen
t.cpp:343
#2  0x00007ffff4a58e36 in DocumentViewerImpl::Destroy (this=0x1baace0)
    at /home/khuey/dev/mozilla-central3/layout/base/nsDocumentViewer.cpp:1647
#3  0x00007ffff578eab9 in nsDocShell::Destroy (this=0x1b523b0)
    at /home/khuey/dev/mozilla-central3/docshell/base/nsDocShell.cpp:4565
#4  0x00007ffff4dc1a45 in nsFrameLoader::Finalize (this=0x1b521a0)
    at /home/khuey/dev/mozilla-central3/content/base/src/nsFrameLoader.cpp:578
#5  0x00007ffff4d9cf89 in nsDocument::MaybeInitializeFinalizeFrameLoaders (
    this=0xd82220)
    at /home/khuey/dev/mozilla-central3/content/base/src/nsDocument.cpp:5497
#6  0x00007ffff4d97219 in nsDocument::EndUpdate (this=0xd82220, aUpdateType=1)
    at /home/khuey/dev/mozilla-central3/content/base/src/nsDocument.cpp:4005
#7  0x00007ffff4fc368a in nsHTMLDocument::EndUpdate (this=0xd82220,
    aUpdateType=1)
    at /home/khuey/dev/mozilla-central3/content/html/document/src/nsHTMLDocument
.cpp:2950
#8  0x00007ffff4b3d001 in mozAutoDocUpdate::~mozAutoDocUpdate (this=
    0x7fffffffa7c0, __in_chrg=<value optimized out>)
    at /home/khuey/dev/mozilla-central3/layout/generic/../../content/base/src/mo
zAutoDocUpdate.h:66
#9  0x00007ffff4ddfb33 in nsINode::doRemoveChildAt (this=0x1af9030, aIndex=2,
    aNotify=1, aKid=0x1b410b0, aChildArray=..., aMutationEvent=1)
    at /home/khuey/dev/mozilla-central3/content/base/src/nsGenericElement.cpp:37
11
#10 0x00007ffff4ddf757 in nsGenericElement::RemoveChildAt (this=0x1af9030,
    aIndex=2, aNotify=1, aMutationEvent=1)
    at /home/khuey/dev/mozilla-central3/content/base/src/nsGenericElement.cpp:36
51
#11 0x00007ffff4de7c2f in nsINode::RemoveChild (this=0x1af9030, aOldChild=
    0x1b410b0) at ../../../dist/include/nsINode.h:485
#12 0x00007ffff56d9fa9 in nsIDOMNode_RemoveChild (cx=0x153bb60, argc=1, vp=
    0x7fffe85630c8) at dom_quickstubs.cpp:6192
...
Comment on attachment 513860 [details] [diff] [review]
Teach nsImageDocument to say the magic password to nsImageLoadingContent::RemoveObserver.

A possible fix is simple enough, but I can't help but fear that we're going to play wack-a-mole with this type of bug.
Attachment #513860 - Flags: review?(jst)
What's the potential size of this leak? I'd like to fix it, but am having trouble deciding if it's bad enough to block. My instincts say "yes, hard blocking final+" - other drivers agree?
About half a MB per iframe destroyed.  I'd agree with hardblocking final.
Well, actually, it's about half a MB for the first one, a smaller amount afterwards (because a lot of the leak is various singletons).
Comment on attachment 513860 [details] [diff] [review]
Teach nsImageDocument to say the magic password to nsImageLoadingContent::RemoveObserver.

Yeah, this is what I was afraid of... I don't see a better way out here though :(

r=jst, this fix is safe, even if it's a whacked mole.
Attachment #513860 - Flags: review?(jst) → review+
I agree with beltzner, final+ hardblocker.
blocking2.0: ? → final+
Whiteboard: [hardblocker]
http://hg.mozilla.org/mozilla-central/rev/a18bb9d573f4
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b12
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.