146549 - Leaking mChildren in nsVoidArray

Status: RESOLVED FIXED

(Core :: Layout, defect, P2)

Description

[Peter A Jonsson]

__builtin_new -:- libstdc++-libc6.1-1.so.2
0x407db1e6 nsSmallVoidArray::SwitchToVe <...> nsVoidArray.cpp:1617 libxpcom.so 
0x402fc52c nsSmallVoidArray::AppendElem <...> nsVoidArray.cpp:1429 libxpcom.so 
0x402fbfea nsReflowPath::EnsureSubtreeF <...> nsReflowPath.cpp:95 libgklayout.so 
0x807091fb IncrementalReflow::AddComman <...> nsPresShell.cpp:1005 libgklayout.so 
0x806e7970 PresShell::ProcessReflowComm <...> nsPresShell.cpp:6348 libgklayout.so 
0x806fa572 ReflowEvent::HandleEvent(void) nsPresShell.cpp:6225 libgklayout.so 
0x808c3cc9 HandlePLEvent(ReflowEvent *) nsPresShell.cpp:6241 libgklayout.so 
0x806fa149 PL_HandleEvent plevent.c:596 libxpcom.so 0x4033f90b 
PL_ProcessEventsBeforeID plevent.c:1270 libxpcom.so 0x40340097

There are more stacktraces at the above URL. Line 1617 in nsVoidArray.cpp at the
time of the run was:
1617    mChildren = (void*)new nsAutoVoidArray();

They don't seem to be free'd at all according to the URL.

Comment 1

[Doug Turner (:dougt)]

neeti can you look at this?

Comment 2

[neeti]

Reporter: how do I look at the stack traces at http://www.geodesic.com/monitor ?

Comment 3

[Peter A Jonsson]

Pick "Select program" in the drop down box in upper left corner. Click on a
binary  and then click on "Leaks & allocations". 

Comment 4

[neeti]

I put a MOZ_COUNT_CTOR(nsSmallVoidArray) and MOZ_COUNT_DTOR(nsSmallVoidArray) 
and from the bloat logs I find all the nsSmallVoidArray being released. 
Also, the destructor of nsSmallVoidArray is deleting the mChildren.

Chris: Could the leak be happening in nsReflowPath.cpp?

Comment 5

[Chris Waterson]

Yes, that would make sense!

Comment 6

[Harshal Pradhan]

I ran into this when investigating bug 181118.

Turns out this happens in debug builds only, and makes it a real pain to find
real leaks. 

Refer
http://lxr.mozilla.org/seamonkey/source/layout/html/base/src/nsReflowPath.cpp#49

The nsReflowPath dtor attempts to mark its memory in an attempt smoke out heap
errors, but doing so at this stage is completely wrong! We have not yet hit the
nsSmallVoidArray dtor for the member mChildren. mChildren member variables thus
get clobbered before they should, causing the nsSmallVoidArray to leak the
nsAutoVoidArray it used to own. Yuck :-(

Comment 7

[Harshal Pradhan]

Attachment: patch (get rid of debug code so that someone else doesn't end up spending time on this)

Comment 8

[Harshal Pradhan]

Might as well take this bug.

Comment 9

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 108845 [details] [diff] [review]
patch (get rid of debug code so that someone else doesn't end up spending time on this)

r=bzbarsky.  One thought... _if_ it's safe to do (and I would assume it's not),
we could do the marking after deleting the reflow path -- the only places that
delete reflow paths are IncrementalReflow::~IncrementalReflow and
nsReflowPath::~nsReflowPath it looks like...

Comment 10

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 108845 [details] [diff] [review]
patch (get rid of debug code so that someone else doesn't end up spending time on this)

sr=dbaron

Code like this belongs in an |operator delete| or in an overridden |free| (but
at the beginning, there).  But it seems fine to just remove this.

Comment 11

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Do you need someone to check this in?

Comment 12

[Harshal Pradhan]

>Do you need someone to check this in?

Yes. Can you take care of it for me please?

Comment 13

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Fix checked in to trunk, 2002-12-15 08:13 PDT.