Closed
Bug 16115
Opened 25 years ago
Closed 25 years ago
we're trying to delete a nsWebShellWindow twice...
Categories
(SeaMonkey :: UI Design, defect, P3)
Tracking
(Not tracked)
VERIFIED
FIXED
People
(Reporter: mscott, Assigned: mscott)
Details
Attachments
(1 file)
799 bytes,
patch
|
Details | Diff | Splinter Review |
In a current pull from 10/11, I'm starting to see assertions in the release method for nsWebShellWindow about us deleting the window twice. I'm assigning this to dp and cc'ing waterson because I'm guessing it may be related to fixing the webshell leak that dp tracked down last week. cc'ing waterson because the solution may involve changes he's going to be making to the tree. Here's the stack trace with comments below: nsDebug::PreCondition(const char * 0x01402380, const char * 0x01402370, const char * 0x0140233c, int 257) line 262 + 13 bytes nsWebShellWindow::Release(nsWebShellWindow * const 0x011abad0) line 257 + 41 bytes nsWebShell::SetDocLoaderObserver(nsWebShell * const 0x011aacd0, nsIDocumentLoaderObserver * 0x00000000) line 1439 + 27 bytes nsWebShell::Destroy(nsWebShell * const 0x011aacd0) line 1146 nsWebShellWindow::~nsWebShellWindow() line 236 nsWebShellWindow::`scalar deleting destructor'() + 15 bytes nsWebShellWindow::Release(nsWebShellWindow * const 0x011abad0) line 257 + 151 bytes nsWebShell::SetDocLoaderObserver(nsWebShell * const 0x011aacd0, nsIDocumentLoaderObserver * 0x00000000) line 1439 + 27 bytes nsWebShell::Destroy(nsWebShell * const 0x011aacd0) line 1146 nsWebShellWindow::Close(nsWebShellWindow * const 0x011abad0) line 439 nsAppShellService::~nsAppShellService() line 145 nsAppShellService::`scalar deleting destructor'(unsigned int 1) + 15 bytes nsAppShellService::Release(nsAppShellService * const 0x0112d5d0) line 153 + 147 bytes When closing a webshell shell window, we are destroying a webshell. Destroying the webshell is causing us to release the last reference on the webshell window so we call into it's dtor which causes us to call Destroy on the webshell again! (all on the same stack trace) This causes a re-entrancy problem which leads to the problems I'm seeing.
Comment 1•25 years ago
|
||
What is the test case. Always on shutdown or anything in particular.
Assignee | ||
Updated•25 years ago
|
Assignee: dp → mscott
Assignee | ||
Comment 2•25 years ago
|
||
Upon further review, I'll go look into fixing this myself so dp doesn't have to worry about it =). At the very least, we should make nsWebshell::Destroy handle the re-entrancy case nicely.
Assignee | ||
Comment 3•25 years ago
|
||
dp, to answer your question, I see this about 50% of the time on shut down on my machine. I can't remember if I've ever seen it if I just run the browser. I know I've seen it when running mailnews and the browser together.
Assignee | ||
Comment 4•25 years ago
|
||
A couple other strange things in here... This is all getting started because the app shell service is getting destroyed. In its dtor, it's closing the hidden window. In closing the hidden window (which is a nsWebShellWidow), we attempt to create the app shell service and unregiser ourself). Of course we're in the dtor of the appshell service so this kind of behavior can't be good either.
Assignee | ||
Comment 5•25 years ago
|
||
I'm now drifting outside of the scope of this bug report. But it looks to me like NS_ShutdownXPCOM is manually deleting all the services (which makes sense since we're trying to cleanup leaks). But shouldn't we be shutting down the services before deleting them? (That sounds vague I know). But in the case of the appshell service, the dtor doesn't do anything involving closing all the open windows. It does have a method on it nsAppShellService::Quit which enumerates over all known windows and asks them to close and shut down. There's also a nsAppShellService::Shutdown which shuts down all the app shell components. It almost seems like we want to be pulling these two methods into the loop before we actually delete the service.
Assignee | ||
Comment 6•25 years ago
|
||
Please ignore my comments about nsAppShellService::Shutdown and nsAppShellService::Quit not being called. I'm sure they are getting called long before we go to destroy the app shell service.
Assignee | ||
Updated•25 years ago
|
Status: NEW → ASSIGNED
Whiteboard: Fix ready...pending code review
Assignee | ||
Comment 7•25 years ago
|
||
Here's a test case I used to reproduce this crash every time: Start up with -mail. Open a browser window. Select File --> Quit. I'm attaching a one line patch to nsWebShellWindow::Close which just does this at the beginning of the method: nsCOMPtr<nsIWebShellWindow> placeHolder = this; This ensures that the web shell window can't get deleted out from under the Close call if the webshell the window destroys is holidng the last reference on the window. dp can you review this for me?
Assignee | ||
Comment 8•25 years ago
|
||
Comment 9•25 years ago
|
||
Patch looks good. Wouldn't the right fix be eliminate the cause for the webshell to turn around and delete the webshellwindow, like removing the webshellwindow from the observer list. What do you think ? If you are convinced that is a longer, tedious approach then fine. Just wanted to make sure you thought of it.
Assignee | ||
Updated•25 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
Whiteboard: Fix ready...pending code review
Assignee | ||
Comment 10•25 years ago
|
||
Hey dp, I just checked in the patch. About your comments. The problem this patch is fixing is the window's close method is destorying a webshell. The webshell has the last outstanding reference to the window so in closing the webshell, we end up destorying the webshell window too. But we're in the middle of the webshell window's close method on the stack. And that's bad. So I don't think there is a more involved solution that would fix this. Thanks for the review!
Updated•24 years ago
|
QA Contact: don → sairuh
Updated•20 years ago
|
Product: Core → Mozilla Application Suite
You need to log in
before you can comment on or make changes to this bug.
Description
•