41988 - nsViewManager2 leaks an nsDrawingSurfaceGTK on shutdown

Status: RESOLVED DUPLICATE of bug 48190

(Core :: Web Painting, defect, P3)

Description

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

DESCRIPTION:  nsViewManager2 leaks an nsDrawingSurfaceGTK because the last 
release of the view manager occurs during XPCOM shutdown.  During XPCOM 
shutdown, nsComponentManager::CreateInstance fails.  This means that |rc| cannot 
be created in |~nsViewManager2|, so |DestroyDrawingSurface| is never called on 
|mDrawingSurface|.

STEPS TO REPRODUCE:
 * use leak logging on "./mozilla -f bloaturls.txt", or look at tinderbox leak 
stats

BUGGY ON:
 * Linux mozilla build of 2000-06-08

ADDITIONAL INFORMATION:
You might be able to just release the drawing surfaces if you can't get a 
rendering context, but I'm not sure how things work on all platforms.

Comment 1

[rusty.lynch]

taking a look into this

Comment 2

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

Adding previous owner back to cc: list.

Comment 3

[rusty.lynch]

Since CreateInstance denies us during shutdown, what if we add another member 
variable for an nsIRenderingContext that is created in the constructor.  Then 
we could use the new member to do the DestroyDrawingSurface's.

I already have it working on my tree.  Is this acceptable?  The additional 
nsIRenderingContext adds a very small about of bloat (96 bytes for the gtk 
version.)

Comment 4

[rusty.lynch]

Attachment: proposed patch for nsViewManager2.cpp and nsViewManager2.h

Comment 5

[Inaky Perez Gonzalez]

Attachment: Proposed fix using XPCOM shutdown observers.

Comment 6

[Inaky Perez Gonzalez]

I´ve attached a patch which fixes this problem using an observer upon XPCOM
shutdown. It is enabled to act only when DEBUG is defined, so that it takes away
the noise but doesn´t get in the way on a release version.

Comment 7

[Inaky Perez Gonzalez]

Attachment: Typo on last patch, please use this one.

Comment 8

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

How is this new patch going to help?  It looks like it still calls
CreateInstance during XPCOM shutdown (in nsViewManager2::xpcom_shutdown).

Comment 9

[Inaky Perez Gonzalez]

Sure it does, but this time we are doing it _before_ closing XPCOM, so the
services are still available, and we can request it. See NS_ShutdownXPCOM() ...
well, at least this makes some sense, and works. This is also the explanation I
got for a fix I proposed to bug #38283.

Comment 10

[Inaky Perez Gonzalez]

Attachment: And last correction (missed freeing correctly the gViewManagers array)

Comment 11

[Inaky Perez Gonzalez]

... on this one (is similar to the statics on #43656 and #43644) I have to use
XPCOM (instead of a refcount, which fails) or as Rusty said, create another RC.
Can we agree on a sollution to check it in?

Comment 12

[Chris Waterson]

add blizzard and pavlov. you two are the GTK gurus; what say you?

Comment 13

[Chris Waterson]

beard: got any preferences/ideas/comments? 

inaky: whichever mechanism we decide to use, this code should *not* be #ifdef
DEBUG.

how many of these objects (nsViewManager2's) get created during the life of the
app? If there is only ever *one*, then let's just keep it around in a member
variable to use on a rainy day. If there are >1, then I'd probably vote for the
XPCOM shutdown hooks (there may be a more elegant way to do this; e.g., at the
module level or something).

Comment 14

[Inaky Perez Gonzalez]

In a fast run using the about:blank page and shutting down, refcnt balancer
reports four objects. Going to slashdot shows five.

Comment 15

[Michael La Guardia]

Cc'ing Valeski. Jud could you make the call about whether this is nsbeta3 + or
-? Thanks.

Comment 16

[Kevin McCluskey (gone)]

I checked in a fix for this bug back on 8/17/2000. If its still leaking a 
drawing surface, it must be for some other reason. I hold a global rendering 
context around just for the purpose of cleaning up the drawing surfaces. When 
the last viewmanager goes away the global rendering context is destroyed as 
well.

Comment 17

[Kevin McCluskey (gone)]

*** This bug has been marked as a duplicate of 48190 ***

Comment 18

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

There are no nsDrawingSurfaceGTK leaks shown in the current speedracer bloat 
stats.  If you know of leaks, please file a separate bug with steps to reproduce 
if one is not already filed.