The default bug view has changed. See this FAQ.

memory leak in Layer with OMTC

RESOLVED FIXED in Firefox 23

Status

()

Core
Graphics: Layers
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: christophe.mouraud, Assigned: nical)

Tracking

Trunk
mozilla23
Points:
---

Firefox Tracking Flags

(blocking-b2g:tef+, firefox21 wontfix, firefox22 wontfix, firefox23 fixed, b2g18 fixed, b2g18-v1.0.0 wontfix, b2g18-v1.0.1 fixed)

Details

(Whiteboard: [MemShrink])

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

4 years ago
Created attachment 731205 [details]
layer.diff

User Agent: Mozilla/5.0 (Windows NT 5.1; rv:20.0) Gecko/20100101 Firefox/20.0
Build ID: 20130320062118

Steps to reproduce:

usign CSS opacity transition  with OMTC


Actual results:

Memroy leak (finding with trace-malloc)
(Reporter)

Comment 1

4 years ago
nsTArray does not own it manages pointers and there are not released in its destructor
OS: Windows XP → All
Hardware: x86 → All
(Assignee)

Comment 2

4 years ago
Hi Christophe, thanks for your patch, can you please format it with the "unified" format, as shown here https://developer.mozilla.org/en-US/docs/Creating_a_patch
and submit it again?

Please add me as reviewer when you do, patches without review request tend to not get much attention.
(Assignee)

Comment 3

4 years ago
Created attachment 734745 [details] [diff] [review]
Fixes ComputedTimingFunction leak (reformated patch)

This fix shouldn't wait longer so I reformated the patch (Christophe, I made sure to put your name and not mine in the patch's header, I don't mean to steal your work).

It's probably worth uplifting asap, I wonder if this could be the cause of the b2g camera app leak.
Assignee: nobody → nical.bugzilla
Attachment #731205 - Attachment is obsolete: true
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #734745 - Flags: review?(jmuizelaar)
(Assignee)

Comment 4

4 years ago
pushed to try severs https://tbpl.mozilla.org/?tree=Try&rev=52f7e9568509
Attachment #734745 - Flags: review?(jmuizelaar) → review+
Whiteboard: [MemShrink]
> +    InfallibleTArray<nsAutoPtr<css::ComputedTimingFunction>>& functions = data->mFunctions;

Shouldn't this be "> >"?  I'd be surprised if all the compilers we support work with ">>" closing the nested template.

How do you think this might cause leaks of the objects identified in bug 846903 comment 64?  I don't see the ComputedTimingFunction class holding on to OGL objects (or any objects at all, actually).
(Assignee)

Comment 6

4 years ago
(In reply to Justin Lebar [:jlebar] from comment #5)
> > +    InfallibleTArray<nsAutoPtr<css::ComputedTimingFunction>>& functions = data->mFunctions;
> 
> Shouldn't this be "> >"?  I'd be surprised if all the compilers we support
> work with ">>" closing the nested template.

Oh, right. here is a fix

> 
> How do you think this might cause leaks of the objects identified in bug
> 846903 comment 64?  I don't see the ComputedTimingFunction class holding on
> to OGL objects (or any objects at all, actually).

This leak happens in SetAnimation which you mentioned in Comment 47 of the b2g bug:
> from mozilla::layers::Layer::SetAnimations(), which may or may not be related.

I suppose this won't fix the leak in the camera entirely though.
(Assignee)

Comment 7

4 years ago
Created attachment 734772 [details] [diff] [review]
Fixes ComputedTimingFunction leak (reformated patch) with >> template syntax fix. Carries jmuizelaar: review+
(Assignee)

Updated

4 years ago
Attachment #734772 - Attachment description: Fixes ComputedTimingFunction leak (reformated patch) with >> template syntax fix → Fixes ComputedTimingFunction leak (reformated patch) with >> template syntax fix. Carries jmuizelaar: review+
(Assignee)

Comment 8

4 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/fdc97a5d6356
https://hg.mozilla.org/mozilla-central/rev/fdc97a5d6356
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
Duplicate of this bug: 861492
tef? because of bug 861492.  Let's land this, please.
blocking-b2g: --- → tef?
status-b2g18: --- → affected
status-b2g18-v1.0.1: --- → affected

Updated

4 years ago
blocking-b2g: tef? → tef+
https://hg.mozilla.org/releases/mozilla-b2g18/rev/eec81d0b7e26
https://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/77bdee6d53aa
status-b2g18: affected → fixed
status-b2g18-v1.0.0: --- → wontfix
status-b2g18-v1.0.1: affected → fixed
status-firefox21: --- → wontfix
status-firefox22: --- → wontfix
status-firefox23: --- → fixed
You need to log in before you can comment on or make changes to this bug.