32313 - MLK: leak of colorspace from DeviceContextImpl

Status: RESOLVED FIXED

(Core :: XUL, defect, P3)

Description

[Bruce Mitchener]

CVS blame points at Troy for this one, but that was way back in 1998.


IL_CreateTrueColorSpace() sets the ref count on the color space to 1.  
DeviceContextImpl::GetILColorSpace() then addrefs it.  That would set the 
refcount to 2.  Even if everything else were properly balanced, the release of 
the colorspace in the destructor of DeviceContextImpl will always end up leaking 
the color space.  Trivial fix would appear to be to remove the addref in 
DeviceContextImpl::GetILColorSpace().


MLK: 12 bytes leaked at 0xbe7fd8
  * This memory was allocated from:
        malloc         [rtlib.o]
        calloc         [rtlib.o]
        PR_Calloc      [prmem.c:48]
        IL_CreateTrueColorSpace [il_util.cpp:358]
        DeviceContextImpl::GetILColorSpace(_NI_ColorSpace*&) 
[nsDeviceContext.cpp:621]
        ImageGroupImpl::Init(nsIDeviceContext*,nsISupports*) 
[nsImageGroup.cpp:195]
        nsPresContext::GetImageGroup(nsIImageGroup**) [nsPresContext.cpp:788]
        nsPresContext::StartLoadImage(const nsString&,const unsigned int*,const 
nsSize*,nsIFrame*,unsigned 
int(*)(nsIPresContext*,nsIFrameImageLoader*,nsIFrame*,void*,unsigned 
int),void*,nsIFrameImageLoader**) [nsPresContext.cpp:853]
        nsHTMLImageLoader::StartLoadImage(nsIPresContext*) 
[nsHTMLImageLoader.cpp:201]
        nsHTMLImageLoader::GetDesiredSize(nsIPresContext*,const 
nsHTMLReflowState*,nsHTMLReflowMetrics&) [nsHTMLImageLoader.cpp:419]
        nsTitledButtonFrame::GetImageSize(nsIPresContext*) 
[nsTitledButtonFrame.cpp:1369]
        nsTitledButtonFrame::GetBoxInfo(nsIPresContext*,const 
nsHTMLReflowState&,nsBoxInfo&) [nsTitledButtonFrame.cpp:1393]
        nsBoxFrame::GetChildBoxInfo(nsIPresContext*,const 
nsHTMLReflowState&,nsIFrame*,nsCalculatedBoxInfo&) [nsBoxFrame.cpp:853]
        nsBoxFrame::GetBoxInfo(nsIPresContext*,const 
nsHTMLReflowState&,nsBoxInfo&) [nsBoxFrame.cpp:2944]
        nsBoxFrame::GetChildBoxInfo(nsIPresContext*,const 
nsHTMLReflowState&,nsIFrame*,nsCalculatedBoxInfo&) [nsBoxFrame.cpp:853]
        nsBoxFrame::GetBoxInfo(nsIPresContext*,const 
nsHTMLReflowState&,nsBoxInfo&) [nsBoxFrame.cpp:2944]
        nsBoxFrame::Reflow(nsIPresContext*,nsHTMLReflowMetrics&,const 
nsHTMLReflowState&,unsigned int&) [nsBoxFrame.cpp:1142]
        
nsContainerFrame::ReflowChild(nsIFrame*,nsIPresContext*,nsHTMLReflowMetrics&,con
st nsHTMLReflowState&,int,int,unsigned int,unsigned int&) 
[nsContainerFrame.cpp:646]
        
nsGfxScrollFrameInner::ReflowFrame(nsIPresContext*,nsHTMLReflowMetrics&,const 
nsHTMLReflowState&,unsigned int&,nsIFrame*,const nsSize&,const 
nsSize&,int&,nsIFrame*&) [nsGfxScrollFrame.cpp:1360]
        nsGfxScrollFrameInner::ReflowScrollbar(nsIPresContext*,const 
nsHTMLReflowState&,unsigned int&,int&,nsIFrame*,nsIFrame*&) 
[nsGfxScrollFrame.cpp:1129]
        nsGfxScrollFrameInner::ReflowScrollbars(nsIPresContext*,const 
nsHTMLReflowState&,unsigned int&,nsIFrame*&) [nsGfxScrollFrame.cpp:1085]
        nsGfxScrollFrame::Reflow(nsIPresContext*,nsHTMLReflowMetrics&,const 
nsHTMLReflowState&,unsigned int&) [nsGfxScrollFrame.cpp:572]
        
nsContainerFrame::ReflowChild(nsIFrame*,nsIPresContext*,nsHTMLReflowMetrics&,con
st nsHTMLReflowState&,int,int,unsigned int,unsigned int&) 
[nsContainerFrame.cpp:646]
        ViewportFrame::Reflow(nsIPresContext*,nsHTMLReflowMetrics&,const 
nsHTMLReflowState&,unsigned int&) [nsViewportFrame.cpp:530]
        PresShell::InitialReflow(int,int) [nsPresShell.cpp:1281]
        HTMLContentSink::StartLayout() [nsHTMLContentSink.cpp:3060]
        HTMLContentSink::OpenBody(const nsIParserNode&) 
[nsHTMLContentSink.cpp:2687]
        CNavDTD::OpenBody(const nsIParserNode*) [CNavDTD.cpp:2642]
        CNavDTD::OpenContainer(const nsIParserNode*,nsHTMLTag,int,nsEntryStack*) 
[CNavDTD.cpp:2894]
        CNavDTD::HandleDefaultStartToken(CToken*,nsHTMLTag,nsIParserNode*) 
[CNavDTD.cpp:1079]

Comment 1

[troy]

Reassigning to Don Cone, because it's a gfx issue. I seem to remember that back 
then sometimes objects had an initial ref count of 0 and sometimes 1. Maybe 
things changed, or maybe I just screwed it up. Who knows, but I'm sure Don will 
fix it. :-)

Comment 2

[dcone (gone)]

It seems that in the constructor for IL_CreateTrueColorSpace the refcount gets 
set to 1, it should be set to 0, and the calling routine will increment the 
refcount if it is going to hold onto it.  The nsDeviceContext is calling 
IL_AddRefToColorSpace which is causing and extra refcount.  I am giving this to 
you Pam.. its just setting the refcount to 0 instead of 1 inside of the 
IL_CreateTrueColorSpace constructor.

Comment 3

[pnunn]

okey doke.
-P

Comment 4

[pnunn]

Looks like the fix will be alittle more complicated.
Just setting the ref_count to 0 creates a new crash.

This will take more study.

Comment 5

[pnunn]

moving to m16 for more testing.
-p

Comment 6

[Gagan]

This is so old a bug! Is it still leaking? how bad? Renominate if its really 
bad. 

Comment 7

[selmer (gone)]

rtm- based on Gagan's comment.  This isn't a stopper unless it's a frequently
hit, very large leak.

Comment 8

[pnunn]

All pnunn bugs reassigned to Pav, who is taking over
the imglib.

Comment 9

[Stuart Parmenter]

yeah, ok, whatever.

Comment 10

[Christopher Hoess (gone)]

Hasn't the imglib rewrite killed this, or am I missing something?

Comment 11

[Stuart Parmenter]

fixed when new imagelib landed