884212 - Leak with nested @media rules

Status: NEW

(Core :: CSS Parsing and Computation, defect)

Description

[Jesse Ruderman]

Attachment: testcase

Comment 1

[Andrew McCreight [:mccr8]]

I can't reproduce a leak.  Is there anything more to this than "open test case, exit browser" to reproduce? I'm a few days behind trunk, so maybe I'm just missing the regression.

Comment 2

[Jesse Ruderman]

Still happens for me on mozilla-central (3b955f306226).

Comment 3

[Jesse Ruderman]

Attachment: leak log

GroupRule, MediaRule, nsMediaList, nsTArray_base

Comment 4

[Andrew McCreight [:mccr8]]

For posterity, the reason I thought I couldn't reproduce the leak is that the little leak summary at the end of a run, when you don't tell it to show you leaks, is just strings, and in this particular case it is a small leak that doesn't leak strings.

Comment 5

[Andrew McCreight [:mccr8]]

I looked at this a little, to try out some fancy new leak investigating tool I'm working on.  According to that, nothing allocated by jemalloc has a reference to the MediaRule, but maybe that's buggy.  I did notice that MediaRule and MediaQueryList should CC the nsMediaLists they hold onto, but that didn't fix the leak.

Comment 6

[Andrew McCreight [:mccr8]]

I did refcount logging on the MediaRule, but it just showed a stack in the CSS parser, which I assume is not very useful.  The code looked reasonable enough.  Maybe I need to make my conservative scanner fuzzier.

Comment 7

[Andrew McCreight [:mccr8]]

I looked at this again, and it is still showing up the same way: one missing reference to the media rule, no pointers to it that I can see in the heap.  That suggests some code somewhere bungling refcounting rather than missing CC traversing.

Comment 8

[brunoais]

Any updates on this matter? Does this memory leak still exist? Was it patched when firefox changed to stylo?

Comment 9

[Andrew McCreight [:mccr8]]

I've looked at this leak here and there since I last posted, but I haven't been able to make any progress on it. I haven't tried since Stylo landed so I don't know if this is still a problem or not.