33160 - MLK: nsSplitterFrameInner leaks some data.

Status: RESOLVED FIXED

(Core :: XUL, defect, P2)

Description

[Bruce Mitchener]

From beard's leak logs.

Appear to leak mChildInfosAfter and mChildInfosBefore.

malloc
operator new(unsigned long, const std::nothrow_t&)
operator new(unsigned long)
__nwa(unsigned long)
nsSplitterFrameInner::MouseDown(nsIDOMEvent*)
nsEventListenerManager::HandleEvent(nsIPresContext*, nsEvent*, nsIDOMEvent**,
unsigned int, nsEventStatus*)
nsXULElement::HandleDOMEvent(nsIPresContext*, nsEvent*, nsIDOMEvent**, unsigned
int, nsEventStatus*)
nsXULElement::HandleDOMEvent(nsIPresContext*, nsEvent*, nsIDOMEvent**, unsigned
int, nsEventStatus*)
PresShell::HandleEvent(nsIView*, nsGUIEvent*, nsEventStatus*, int&)
nsView::HandleEvent(nsGUIEvent*, unsigned int, nsEventStatus*, int&)
nsView::HandleEvent(nsGUIEvent*, unsigned int, nsEventStatus*, int&)
nsViewManager2::DispatchEvent(nsGUIEvent*, nsEventStatus*)
HandleEvent(nsGUIEvent*)
nsWindow::DispatchEvent(nsGUIEvent*, nsEventStatus&)
nsWindow::DispatchWindowEvent(nsGUIEvent&)
nsWindow::DispatchMouseEvent(nsMouseEvent&)
nsMacEventHandler::HandleMouseDownEvent(EventRecord&)
nsMacEventHandler::HandleOSEvent(EventRecord&)
nsMacWindow::HandleOSEvent(EventRecord&)
nsMacMessageSink::DispatchOSEvent(EventRecord&, GrafPort*)
nsMacMessagePump::DispatchOSEventToRaptor(EventRecord&, GrafPort*)
nsMacMessagePump::DoMouseDown(EventRecord&)
nsMacMessagePump::DispatchEvent(int, EventRecord*)
nsMacMessagePump::DoMessagePump()
nsAppShell::Run()
nsAppShellService::Run()
main1(int, char**, nsISplashScreen*)
main
__NSInitialize(1F1D7564)

Comment 1

[Mike Shaver (:shaver emeritus)]

I _think_ this might just be because we're not deleting with the right
operator.  Patch attached, I'm interested in commentary from bruce and beard.

Comment 2

[Mike Shaver (:shaver emeritus)]

Attachment: Fix: use delete[] on mChildInfos{Before,After}

Comment 3

[Bruce Mitchener]

I don't know anything about the mechanics behind beard's leak tools to tell 
whether or not that is the problem.  If anyone has a good idea on how to 
reproduce this, preferably inside of viewer, I'd be happy to test.

Comment 4

[Patrick C. Beard]

While delete[] is always the right thing to do with arrays of objects, but the 
real cause of this leak is that ~nsSplitterFrameInner() doesn't do the final 
delete[], and your patch just fixes the case when mChildInfosBefore/After are 
reallocated, if I understand the code correctly. So, we need some extra code 
inserted into ~nsSplitterFrameInner() to do the final delete[].

Comment 5

[Patrick C. Beard]

Attachment: Complete patch to fix the leak.

Comment 6

[Peter Trudelle]

Thanks guys, you rock! reassigning to evaughan for review/checkin.

Comment 7

[Bruce Mitchener]

To fix all of the FMM errors (delete vs delete[]), you'll also need to look at 
the other uses of ChildInfos in the file, like, nsSplitterFrameInner::Reverse().   
Looking at the file, that was the only other use that I saw.

Comment 8

[Eric Vaughan]

I have this change made in my current build. I'll land it soon with my box carpool.

Comment 9

[Bruce Mitchener]

Do you have an estimate of when that will happen so that I know when to watch
for it?  Thanks!

Comment 10

[Eric Vaughan]

fixed

Comment 11

[Patrick C. Beard]

The destructor fix wasn't in. I just added it.

Comment 12

[Bruce Mitchener]

the problem with nsSplitterFrameInner::Reverse() wasn't fixed in the patch and
still remains in the tree. Reopening.

Comment 13

[Peter Trudelle]

Mass-moving most M15 bugs to M16

Comment 14

[Eric Vaughan]

fixed. Reverse now cleans itself up.