Last Comment Bug 757749 - Leak with canvas, addEventListener, matchMedia
: Leak with canvas, addEventListener, matchMedia
Status: RESOLVED FIXED
[MemShrink:P2]
: mlk, testcase
Product: Core
Classification: Components
Component: Canvas: 2D (show other bugs)
: Trunk
: x86_64 Mac OS X
: -- normal (vote)
: ---
Assigned To: Nobody; OK to take it and work on it
:
Mentors:
Depends on: 731868
Blocks: 326633 379903
  Show dependency treegraph
 
Reported: 2012-05-22 23:46 PDT by Jesse Ruderman
Modified: 2012-06-23 05:50 PDT (History)
9 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
testcase (386 bytes, text/html)
2012-05-22 23:46 PDT, Jesse Ruderman
no flags Details
ref count tree, with balanced subtrees ignored (48.34 KB, text/plain)
2012-05-24 17:25 PDT, Andrew McCreight [:mccr8]
no flags Details
refcount tree (108.96 KB, text/plain)
2012-05-24 17:54 PDT, Andrew McCreight [:mccr8]
no flags Details

Description Jesse Ruderman 2012-05-22 23:46:23 PDT
Created attachment 626343 [details]
testcase

According to trace-refcnt, this testcase makes Firefox leak nsGlobalWindow objects.
Comment 1 Boris Zbarsky [:bz] (Out June 25-July 6) 2012-05-23 12:16:27 PDT
I just tried this with a slightly old build (from rev fb6109c9dae5) and that doesn't leak...

I'll try updating.
Comment 2 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2012-05-23 12:32:30 PDT
I can't reproduce this on linux.
Comment 3 Boris Zbarsky [:bz] (Out June 25-July 6) 2012-05-23 12:34:46 PDT
OK, rev a4240610972e does show the leak.
Comment 4 Boris Zbarsky [:bz] (Out June 25-July 6) 2012-05-23 20:18:19 PDT
And now I see it with rev fb6109c9dae5 too.

My first guess would be that we end up with a cycle through the prescontext somehow, but I dunno exactly how...
Comment 5 Andrew McCreight [:mccr8] 2012-05-24 16:49:12 PDT
A canvas element is being kept alive here somehow.  My money is on bungled refcounting in nsCanvasRenderingContext2DAzure. ;)

0x12626ded0 [nsGenericElement (xhtml) canvas https://bug757749.bugzilla.mozilla.org/attachment.cgi?id=626343]
    --[[via hash] mListenerManager]-> 0x12674f310 [nsEventListenerManager]
    --[mListeners[i] mListener]-> 0x12549b410 [nsXPCWrappedJS (nsIDOMEventListener)]
    --[mJSObj]-> 0x1184e58c0 [JS Object (Function)]
    --[script]-> 0x1184740e8 [JS Script]
    --[object]-> 0x118470060 [JS Object (Window)]
    --[xpc_GetJSPrivate(obj)]-> 0x126f42ba0 [XPCWrappedNative (Window)]
    --[mIdentity]-> 0x1264d5080 [nsGlobalWindow]

    Root 0x12626ded0 is a ref counted object with 1 unknown edge(s).
    known edges:
       0x126697bc0 [nsGenericElement (xhtml) body https://bug757749.bugzilla.mozilla.org/attachment.cgi?id=626343] --[mAttrsAndChildren[i]]-> 0x12626ded0
       0x1264d3000 [nsCanvasRenderingContext2DAzure] --[mCanvasElement]-> 0x12626ded0
Comment 6 Andrew McCreight [:mccr8] 2012-05-24 17:25:04 PDT
Created attachment 627042 [details]
ref count tree, with balanced subtrees ignored
Comment 7 Andrew McCreight [:mccr8] 2012-05-24 17:54:21 PDT
Created attachment 627053 [details]
refcount tree
Comment 8 Boris Zbarsky [:bz] (Out June 25-July 6) 2012-05-24 18:33:02 PDT
The first refcount log shows outstanding refs from the canvas context (expected: they form a cycle), the parent element in the DOM, and what looks like GetCanvasLayer (via CanvasRenderingContext2DUserData)

Is it possible that the canvas layer is leaking for some reason?
Comment 9 Andrew McCreight [:mccr8] 2012-05-24 22:02:50 PDT
(In reply to Boris Zbarsky (:bz) from comment #8)
> Is it possible that the canvas layer is leaking for some reason?
It could be, but it must not be in the CC graph.  That means either that it isn't a CCed class, or it was erroneously dropped.
Comment 10 Kyle Huey [:khuey] (khuey@mozilla.com) 2012-05-24 22:03:46 PDT
I really hope layers aren't CCd objects!
Comment 11 Boris Zbarsky [:bz] (Out June 25-July 6) 2012-05-24 22:06:46 PDT
The layers aren't CCed.
Comment 12 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2012-05-25 04:00:09 PDT
So isn't this then a dup of ... the one bug where prescontext ends up keeping layers alive
too long.
Comment 13 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2012-05-25 04:00:34 PDT
(or was that fixed?)
Comment 14 Andrew McCreight [:mccr8] 2012-05-25 07:53:26 PDT
(In reply to Olli Pettay [:smaug] from comment #12)
> So isn't this then a dup of ... the one bug where prescontext ends up
> keeping layers alive too long.
Do you have a bug number for that?
Comment 15 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2012-05-25 08:07:19 PDT
I was thinking Bug 731868
Comment 16 Andrew McCreight [:mccr8] 2012-05-25 09:22:59 PDT
That does sound very similar.
Comment 17 Andrew McCreight [:mccr8] 2012-05-25 16:27:42 PDT
There are (In reply to Boris Zbarsky (:bz) from comment #8)
> Is it possible that the canvas layer is leaking for some reason?
There are 8 Layers leaking.  I'm not sure which is which, though.
Comment 18 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-05-28 19:52:45 PDT
I can't reproduce this. Anyone got exact steps to reproduce?
Comment 19 Andrew McCreight [:mccr8] 2012-05-28 19:56:57 PDT
I'm able to reproduce it by just opening the browser with the test case loading as the initial tab, then exiting the browser.  Maybe it is OSX only or something?
Comment 20 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-05-28 20:09:42 PDT
Windows and GTK2 nsWindow::Destroy are doing stuff like this:
  if (mLayerManager) {
    mLayerManager->Destroy();
  }
  mLayerManager = nsnull;

I'm guessing nsChildView::Destroy needs that somewhere too. I don't think that's happening right now.
Comment 21 Andrew McCreight [:mccr8] 2012-05-29 09:42:47 PDT
I tried blindly putting that chunk of code into nsChildView::Destroy in a few places but it didn't fix the leak.  Somebody who actually knows some of this code should probably take over from here.
Comment 22 Andrew McCreight [:mccr8] 2012-05-29 10:05:04 PDT
Ignore the first sentence of the previous comment.  I was managing to edit a different directory from the one I was running.  When I tried it for real, I ended up crashing in the cycle collector on shutdown while tearing down this view stuff:

mozilla::gl::TextureImageCGL::~TextureImageCGL() + 71 (GLContext.h:583)
nsChildView::~nsChildView() + 357 (nsChildView.mm:246)
nsBaseWidget::Release() + 282 (nsBaseWidget.cpp:56)
nsChildView::Release() + 24 (nsChildView.mm:251)
nsPresContext::cycleCollection::Unlink(void*) + 1102 (mozalloc.h:224)

~nsChildView() has some comment about doing some cleanup in case Destroy() hasn't been called, so presumably my random nulling out has upset something there.
Comment 23 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-06-23 05:50:41 PDT
Fixed in bug 731868. CanvasRenderingContext2DUserData(Azure) no longer has an owning reference to anything.

Note You need to log in before you can comment on or make changes to this bug.