39356 - ViewManager2 leaks a DrawingSurface

Status: RESOLVED INVALID

(Core Graveyard :: Embedding: GTK Widget, defect, P3)

Description

[yueheng.xu]

I run the refcnt balancer and found out that the leaked class 
nsDrawingSurfaceGTK as reported in the bloatview is actually is:

The nsViewManager2:Refresh() defined a local variable ds

 nsDrawingSurface ns = null;              

( line 454 in 
http://lxr.mozilla.org/mozilla/source/view/src/nsViewManager2.cpp )

 this ds created a new object at line 514:

    ds = GetDrawingSurface(...)

 and used in line 529 and line 530. But it never release the object.


Fix:  add  NS_IF_RELEASE(ds) in nsViewManager2:refresh() when ds is no longer
      needed.

Comment 1

[Hoa Nguyen]

yueheng, small little typo:

   nsDrawingSurface ds = null;

instead of:

   nsDrawingSurface ns = null;

More importantly, it doesn't appear from the code as if nsDrawingSurface is an 
XPCOM object, I'm not sure your suggestion fixes anything (or is even valid to 
release this object).  From glancing at the code, seems like ds is obtained 
from GetDrawingSurface, which in turn returns a member variable 
(mDrawingSurface), which in turn gets destroyed in the destructor.

Comment 2

[rusty.lynch]

For each CreateDrawingSurface there needs to be a DestroyDrawingSurface.  As 
far as I can tell, everytime a new drawing surface is created in
nsViewManager2.cpp (for mDrawingSurface, gOffScreen, gRed, or gBlue) a check is 
made to destroy any preexisting surface.  And then, in the
nsViewManager2's destructor, each of the four possible surfaces is checked and 
DestroyDrawingSurface is called on each if needed.

I don't see a leak here, and calling NS_IF_RELEASE(ds) will probobly cause a 
crash.

Comment 3

[yueheng.xu]

It seems that above comments are convincing. Here I attach the ref count 
balance tree for future reference if we want to go back to this.

Comment 4

[yueheng.xu]

Attachment: Refcnt Balance tree