755664 - CompositorParent seems leaking mCurrentCompositeTask and CompositorParent in Tuple

Status: RESOLVED INVALID

(Core :: Graphics: Layers, defect)

Description

[Oleg Romashin (:romaxa)]

I was chasing weird memory leak, and found that CompositorParent not destroying after window close.
Reason is mCurrentCompositeTask which we allocate in 
CompositorParent::ScheduleComposition
mCurrentCompositeTask = NewRunnableMethod(this, &CompositorParent::Composite);

NewRunnableMethod create Tuple and addreffing this-CompositorParent.

In CompositorParent::Composite we don't delete that task and just set it to NULL, which is causing Tuple leak and CompositorParent refCount increase on every paint...


Checked other cancelable tasks across mozilla code, and noticed that we never delete them too.... am I misunderstood something or we really leak tasks?

Comment 1

[Oleg Romashin (:romaxa)]

Attachment: Delete cancalable task in Compositor

Comment 2

[Ali Juma [:ajuma]]

Comment on attachment 624330 [details] [diff] [review]
Delete cancalable task in Compositor

(In reply to Oleg Romashin (:romaxa) from comment #0)

> Checked other cancelable tasks across mozilla code, and noticed that we
> never delete them too.... am I misunderstood something or we really leak
> tasks?

Tasks get deleted after they are run in MessageLoop::RunTask (see http://mxr.mozilla.org/mozilla-central/source/ipc/chromium/src/base/message_loop.cc#319), so we shouldn't be deleting tasks ourselves after they get run.

> I was chasing weird memory leak, and found that CompositorParent not
> destroying after window close.
> Reason is mCurrentCompositeTask which we allocate in 
> CompositorParent::ScheduleComposition
> mCurrentCompositeTask = NewRunnableMethod(this,
> &CompositorParent::Composite);
> 
> NewRunnableMethod create Tuple and addreffing this-CompositorParent.
> 
> In CompositorParent::Composite we don't delete that task and just set it to
> NULL, which is causing Tuple leak and CompositorParent refCount increase on
> every paint...

It's possible that at shutdown, there's a pending mCurrentCompositeTask that's getting deleted too late. If this is the problem, something like the following should fix the leak. In CompositorParent::RecvWillStop(), add:

if (mCurrentCompositeTask) {
   mCurrentCompositeTask->Cancel();
   delete mCurrentCompositeTask;
   mCurrentCompositeTask = NULL;
}

And in CompositorParent::ScheduleComposition(), return immediately if mPaused (so that after RecvWillStop, we don't create more tasks).

Comment 3

[Oleg Romashin (:romaxa)]

ah, ok... I got it, task is not deleted, beause I never run it.
I added task handling to EmbedCompositor implementation, and that is fixing all problems

Comment 4

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

We should be using ScopedRunnableMethodFactory for this.  See dom/plugins/ipc/PluginModuleParent.{h,cpp} for examples.