Closed
Bug 43590
Opened 24 years ago
Closed 24 years ago
nsScreenWin held past XPCOM shutdown in static nsCOMPtr
Categories
(Core Graveyard :: GFX, defect, P3)
Tracking
(Not tracked)
VERIFIED
FIXED
mozilla0.8
People
(Reporter: dbaron, Assigned: mikepinkerton)
References
(Blocks 1 open bug)
Details
(Keywords: embed, memory-leak, topembed+, Whiteboard: [nsbeta3-])
Attachments
(4 files)
2.75 KB,
patch
|
Details | Diff | Splinter Review | |
2.75 KB,
patch
|
Details | Diff | Splinter Review | |
80.82 KB,
text/plain
|
Details | |
81.11 KB,
text/plain
|
Details |
An nsScreenWin is held past XPCOM shutdown in the "static nsCOMPtr<nsIScreen> sPrimaryScreen" defined at: http://lxr.mozilla.org/seamonkey/source/gfx/src/windows/nsDeviceContextWin.cpp#2 39 See bug 43561 for more info.
Updated•24 years ago
|
Status: NEW → ASSIGNED
Target Milestone: --- → M21
Comment 1•24 years ago
|
||
Don, could you take a look at this.
Assignee: kmcclusk → dcone
Status: ASSIGNED → NEW
Updated•24 years ago
|
Comment 3•24 years ago
|
||
Correction: Marking nsbeta3-
Comment 4•24 years ago
|
||
Marking nsbeta3+. Reassigning to Waqar
Assignee: dcone → waqar
Status: ASSIGNED → NEW
Whiteboard: [nsbeta3-] → [nsbeta3+]
Comment 5•24 years ago
|
||
The problem is the primary screen is cached in FindScreen but it is never released. May want to move the code to cache the sPrimaryScreen into the screen manager's GetPrimaryScreen, and release the cached primary screen in the Screen Manager's dtor. CC'ing pinkerton since he was the original owner of this code.
Comment 6•24 years ago
|
||
Looks like Mac has the same problem: http://lxr.mozilla.org/seamonkey/source/gfx/src/mac/nsDeviceContextMac.cpp#380 http://lxr.mozilla.org/seamonkey/source/gfx/src/mac/nsDeviceContextMac.cpp#473 May also want to consider avoiding caching the primary screen if its not a significant performance hit. Reassigning to pinkerton@netscape.com Pink: If your overloaded, I can look it this further.
Assignee: waqar → pinkerton
Comment 7•24 years ago
|
||
clearing nsbeta3+ for triage by new owners. Need info: Does anything bad happen due to this that will affect users?
Whiteboard: [nsbeta3+] → [need info]
Comment 8•24 years ago
|
||
Kevin, could you help us triage this? We don't understand why it should be on our short list of bugs to fix for 9/14.
Comment 9•24 years ago
|
||
The nomination came from the embedding team. It was part of their list of top footprint/memory bugs. I think the only real issue with this bug is that we are probably leaking a few device handles on WIN95/98 during application shutdown which may not be returned to the system. I suggest marking nsbeta3- at this point.
Comment 11•24 years ago
|
||
can't we make this a static raw ptr and let some service w/ scope release it when it goes away?
Updated•24 years ago
|
Whiteboard: [nsbeta3-]
Comment 12•24 years ago
|
||
shouldn't affect near-term embedding clients, all of which use Linux. nsbeta3-/future
Whiteboard: [nsbeta3-]
Assignee | ||
Updated•24 years ago
|
Target Milestone: mozilla0.9 → mozilla0.8
Reporter | ||
Comment 13•24 years ago
|
||
Comment 14•24 years ago
|
||
it does not compile :-(( C:\moz_sour\mozilla\mozilla\gfx\src\windows>nmake -f makefile.win Microsoft (R) Program Maintenance-Dienstprogramm: Version 6.00.8168.0 Copyright (C) Microsoft Corp 1988-1998. Alle Rechte vorbehalten. +++ make: exporting headers nsDeviceContextWin.cpp C:\moz_sour\mozilla\mozilla\gfx\src\windows\nsDeviceContextWin.cpp(100) : error C2724: 'Shutdown' : 'static' sollte nicht fuer Member-Funktionen verwendet werde n, die ausserhalb der Klasse definiert werden NMAKE : fatal error U1077: 'C:\PROGRA~1\MICROS~2\VC98\BIN\cl.exe' : Rueckgabe-Co de '0x2' Stop. NMAKE : fatal error U1077: '"C:\PROGRAMME\MICROSOFT VISUAL STUDIO\VC98\BIN\NMAKE .exe"' : Rueckgabe-Code '0x2' Stop. translation: static should not be used for a member functions, which are declared outside of the class
Reporter | ||
Comment 15•24 years ago
|
||
Comment 16•24 years ago
|
||
Comment 17•24 years ago
|
||
The pacth compiles, it fixes the assertion under Win98! Thank you David.
Comment 18•24 years ago
|
||
couple questions: 1. What assertion? 2. Can you post a before and after bloat/leak log (sorry to nit)? 3. You mention that "we leak" in your comment at the bottom of the patch. Can you ellaborate on that? Or better yet, whack this to be an nsIModule?
Reporter | ||
Comment 19•24 years ago
|
||
Either Win98 runs static dtors on a different thread or NS_GetCurrentThread returns the wrong thread while running static dtors, so threadsafety assertions from the NS_IMPL_ADDREF/NS_IMPL_RELEASE are triggered on shutdown while releasing static nsCOMPtrs. It's really annoying when developing on Win98, especially for things like the layout regression tests. I'm not sure whether we should prefer leaks or static nsCOMPtrs. Of course, I'd rather see neither. This patch turns the static nsCOMPtr into a leak because I wanted to produce a quick patch that would shut up the assertions for those who want that.
Comment 20•24 years ago
|
||
Comment 21•24 years ago
|
||
It does not introduce a new leak, nsScreenWin is also leaked without the patch at least under Win98. I use the patch, it works and do not need to press away all the assertions anymore.
Assignee | ||
Comment 22•24 years ago
|
||
fixed.
Status: NEW → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
Updated•16 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•