nsViewManager2 leaks an nsDrawingSurfaceGTK on shutdown

RESOLVED DUPLICATE of bug 48190

Status

()

Core
Layout: View Rendering
P3
normal
RESOLVED DUPLICATE of bug 48190
18 years ago
17 years ago

People

(Reporter: dbaron, Assigned: rusty.lynch)

Tracking

({mlk})

Trunk
x86
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [tind-mlk][need info])

Attachments

(4 attachments)

(Reporter)

Description

18 years ago
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.
(Reporter)

Updated

18 years ago
Keywords: mlk

Updated

18 years ago
Status: NEW → ASSIGNED
Target Milestone: --- → M21
(Reporter)

Updated

18 years ago
Whiteboard: [tind-mlk]
(Assignee)

Comment 1

18 years ago
taking a look into this
Assignee: kmcclusk → rusty.lynch
Status: ASSIGNED → NEW
(Reporter)

Comment 2

18 years ago
Adding previous owner back to cc: list.
(Assignee)

Comment 3

18 years ago
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.)

(Assignee)

Comment 4

18 years ago
Created attachment 10188 [details] [diff] [review]
proposed patch for nsViewManager2.cpp and nsViewManager2.h

Comment 5

18 years ago
Created attachment 10483 [details] [diff] [review]
Proposed fix using XPCOM shutdown observers.

Comment 6

18 years ago
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.
Keywords: patch

Comment 7

18 years ago
Created attachment 10488 [details] [diff] [review]
Typo on last patch, please use this one.
(Reporter)

Comment 8

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

Comment 9

18 years ago
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

18 years ago
Created attachment 10519 [details] [diff] [review]
And last correction (missed freeing correctly the gViewManagers array)

Comment 11

18 years ago
... 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

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

Comment 13

18 years ago
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

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

Updated

18 years ago
Keywords: nsbeta3

Comment 15

17 years ago
Cc'ing Valeski. Jud could you make the call about whether this is nsbeta3 + or
-? Thanks.
Whiteboard: [tind-mlk] → [tind-mlk][need info]
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.

*** This bug has been marked as a duplicate of 48190 ***
Status: NEW → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → DUPLICATE
(Reporter)

Comment 18

17 years ago
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.
You need to log in before you can comment on or make changes to this bug.