*Might* be fallout from bug 297561, but I'm not sure. Build ID: 2005-08-14-05, Windows XP SeaMonkey trunk. Summary: "Opening [filename.ext]" dialog won't close (GDI leak?) Steps to Reproduce: 1. In a current SeaMonkey build, load http://www.yolinux.com/TUTORIALS/LinuxTutorialMimeTypesAndApplications.html 2. Click on http://www.yolinux.com/MIME-EXAMPLES/launch.mpg (or any file linked) 3. After opening/saving the file, try to close the window Expected Results: Should be able to close the window. Actual Results: Window is unable to be closed, and appears that GDI resources leak (screen paints through). See screenshot.
Builds up to and including 2005-08-13-05 do NOT have this bug, builds later (i.e. 2005-08-14-05) do.
can't reproduce, BuildID 2005081405 from zip package Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.9a1) Gecko/20050814 SeaMonkey/1.0a 1. http://www.yolinux.com/TUTORIALS/LinuxTutorialMimeTypesAndApplications.html Didn't find the mpg link on that page 2. http://www.yolinux.com/MIME-EXAMPLES/launch.mpg left-clicked that URL here, and download started automatically, afterwards Windows MediaPlayer came up. saved other files by right-click, file box closed automatically or on abort.
wfm Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.9a1) Gecko/20050814 SeaMonkey/1.0a found that link by scrolling just a tad to the right, didn't see the last column of the table in my first comment. Left click, right click, all is working fine.
Okay, sorry, better steps: 1. Click http://www.yolinux.com/MIME-EXAMPLES/launch.mpg, but DO NOT click OK (i.e. just let the dialog sit there in the "choose" state) 2. Now click on http://www.yolinux.com/MIME-EXAMPLES/video.avi, but BEFORE you click OK to "Open it with the current application (Winamp)", position it in the bottom right of the other dialog. (Make sure both dialogs are visible on screen, but the video.avi (which you will perform step #3 on) is on TOP of the launch.mpg dialog.) 3. Now, click OK on the first dialog (launch.mpg), and watch the fun begin.
Also wfm with 20050814 trunk SeaMonkey build. I've tried to reproduce with the steps in comment 5.
13 years ago
I'm able to reproduce. Investigating..
I was able to reproduce this a couple of times. Couldn't find anything supporting the theory that bug 297561 has something to do with this. For some reason window.close() just destroys the XUL stuff but doesn't close the window. Now I'm not able to reproduce again. Anyway, this is a bit out of my scope unless someone can say something definite about how bug 297561 is related.
Created attachment 192753 [details] Crash stack For what it's worth, it seems after trying to reproduce this I'm often getting a crash upon shutdown. It could be related as download manager is involved.
(In reply to comment #9) > For what it's worth, it seems after trying to reproduce this I'm often getting > a crash upon shutdown. It could be related as download manager is involved. Same for me, I crash only at shutdown after reproduce comment 5. See Talkback TB8406759M.
I can't reproduce this one anymore nor do I crash upon shutdown.
Oops, I do crash, but can't reproduce the dialog bug.
The crash happens because DownloadManager dies before Download, but Download calls DownloadManager in the destructor. Anyone interested in fixing this?
(In reply to comment #12) > Oops, I do crash, but can't reproduce the dialog bug. Yeah, I can't reproduce any longer with build 2005-08-22-05 on Windows XP SeaMonkey trunk either. As for the crash, I'm totally fine with morphing this bug to cover it, instead.
worksforme with linux seamonkey trunk 2005082703 so this is a download manager bug?
(In reply to comment #13) > The crash happens because DownloadManager dies before Download, but Download > calls DownloadManager in the destructor. Anyone interested in fixing this? Is it as simple as checking the validity of mDownloadManager before calling its assert? (And maybe having ~nsDownloadManager clear mDownloadManager in all the nsDownloads?) http://lxr.mozilla.org/seamonkey/source/xpfe/components/download-manager/src/nsDownloadManager.cpp#913
Tested on a recent Mac branch nightly and couldn't reproduce. Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b4) Gecko/20050907 SeaMonkey/1.0a
(In reply to comment #16) > Is it as simple as checking the validity of mDownloadManager before calling its > assert? (And maybe having ~nsDownloadManager clear mDownloadManager in all the > nsDownloads?) I'm not sure. Of course that would prevent this symptom, but the cause might be that the downloads should have been destroyed already earlier or something.
I can reproduce this bug pretty easily with my gtk2 branch builds (I think... I can't get a full stack, and it also occurs using progress dialogs).
*** Bug 308646 has been marked as a duplicate of this bug. ***
I only get this on trunk builds BuildID 2005091007 and later (see duped bug for details)
I finally got a reasonable stack out of my branch builds, and it's the same as attachment 192753 [details], even if I have both progress dialogs and download manager disabled. Also, I verified with valgrind that my other builds are also seeing this bug (access the download manager after it's been deleted), but they just don't crash.
(In reply to comment #16) > (In reply to comment #13) > > The crash happens because DownloadManager dies before Download, but Download > > calls DownloadManager in the destructor. Anyone interested in fixing this? > > Is it as simple as checking the validity of mDownloadManager before calling its > assert? (And maybe having ~nsDownloadManager clear mDownloadManager in all the > nsDownloads?) > > http://lxr.mozilla.org/seamonkey/source/xpfe/components/download-manager/src/nsDownloadManager.cpp#913 Adding if (mDownloadManager) into there does not seem to resolve the crashing.
Check out dbaron's comments here: https://bugzilla.mozilla.org/show_bug.cgi?id=237736#c9
> http://lxr.mozilla.org/seamonkey/source/xpfe/components/download-manager/src/nsDownloadManager.cpp#913 > > Adding if (mDownloadManager) into there does not seem to resolve the crashing. Which is to be expected because there's nothing that would nullify it when the download manager is destroyed.
The patch for bug 304563 might fix this too. It probably caused the download window not to close completely.
(In reply to comment #26) > The patch for bug 304563 might fix this too. It probably caused the download > window not to close completely. You meant bug 307563, adding autoScroll shouldn't have any effect on this crasher ;) Can this bug still be seen with builds containing the patch for bug 307563? That patch was checked in on 2005-10-15 11:18.
Bug 307563 certainly wasn't causing the crash for me on linux. I'm not seeing the crash with current trunk and branch builds, but I also don't see the bug with older builds that did crash. Last I checked the talkback URL (the server seems to be down now), it still had current crashes.
SeaMonkey 1.0 Beta is frozen, and we still have no clue what's happening, also this seems to be happening very rarely and not easily reproducable, so not blocking beta for this.
email@example.com: downloadmanager should observe xpcom shutdown and drop everything then. not later. (actually, so should nsExternalAppHandler).
*** Bug 340115 has been marked as a duplicate of this bug. ***
Created attachment 241802 [details] [diff] [review] give nsDownload an owning ref to nsDownloadManager This keeps the download manager alive so long as there is a download. nsDownloadManager removes its refs to the nsDownloads via ::DownloadEnded
this is actually strange come to think of it, because the externalapphandler should not hold a ref to the nsDownload by the time it gets destructed...
Right, one might expect nsExternalAppHandler::CloseWindow, ::Cancel or ::OnStopRequest to be called before ~nsExternalAppHandler, but that isn't happening. It's hard to tell from the stack how it got into this situation. Anyway, the attached patch should a) fix the crash and b) is correct since nsDownload depends on nsDownloadManager existing as long as the nsDownload does.
so, the problem is that there's a js object which held a reference to exthandler xpcom_core.dll!PL_DHashTableOperate(PLDHashTable * table=0x0451b760, const void * key=0x0013f7a4, PLDHashOperator op=PL_DHASH_LOOKUP) Line 490 + 0xd C docshell.dll!nsExternalAppHandler::~nsExternalAppHandler() Line 1390 + 0x2a C++ docshell.dll!nsExternalAppHandler::`scalar deleting destructor'() + 0xf C++ docshell.dll!nsExternalAppHandler::Release() Line 1343 + 0x8e C++ This object is being GCd because some js held a reference to it: xpc3250.dll!XPCJSRuntime::GCCallback(JSContext * cx=0x0439c840, JSGCStatus status=JSGC_END) Line 562 + 0x12 C++ That js world is being destroyed: js3250.dll!JS_DestroyContext(JSContext * cx=0x0439c840) Line 943 + 0xb C The js was in the global scope for a xul document, which goes away: gklayout.dll!nsXULPDGlobalObject::SetContext(nsIScriptContext * aContext=0x00000000) Line 811 C++ gklayout.dll!nsXULPrototypeDocument::~nsXULPrototypeDocument() Line 271 C++ because the xul document is going away: gklayout.dll!nsXULDocument::~nsXULDocument() Line 444 + 0x46 C++ gklayout.dll!nsXULDocument::Release() Line 476 + 0xc C++ because the last js reference to the xul window was GC'd: js3250.dll!js_GC(JSContext * cx=0x00aa3ba0, unsigned int gcflags=0) Line 1947 + 0xf C because a js xpcom component is going away: js3250.dll!JS_DestroyContext(JSContext * cx=0x00aa3ba0) Line 943 + 0xb C because xpconnect is shutting down xpc3250.dll!mozJSComponentLoader::UnloadAll(int aWhen=3) Line 1007 + 0xd C++ because everything's unloading xpcom_core.dll!nsComponentManagerImpl::UnloadLibraries(nsIServiceManager * serviceMgr=0x00000000, int aWhen=3) Line 3114 + 0x28 C++ because xpcom is shutting down seamonkey.exe!NS_ShutdownXPCOM(nsIServiceManager * servMgr=0x00000000) Line 171 + 0xa C++ The root problem in a way is that some js xpcom component is holding a reference to a xul window which it probably shouldn't be doing. that could be your download manager component i suppose. this is why you don't see closewindow on the stack, the close window call would have happened, but the js reference kept the window alive. some weak references might be a good idea.
Should this go into SeaMonkey 1.1 (on 1.8 branch)? If so, please nominate it for that by requesting approval-seamonkey1.1b? once it has reviews and fixes the crash on trunk. This is no really major problem though, it's just annoying, so we won't push back a beta for it, we can still take such stability fixes (without L10n changes) after Beta and still have a more stable final.
I still haven't found out which window (tricky to do in destructors), but I did find two references to button.xml in my latest stack.
(In reply to comment #35) > so, the problem is that there's a js object which held a reference to > exthandler That's not the entire answer. exthandler should still have dropped its reference to the download earlier. It seems kind of odd that the reference cycle here was broken (or these two objects couldn't be destructed) but the exthandler->download reference wasn't dropped earlier.
11 years ago
I'd still like to know how this was possible though...
(In reply to comment #39) >I'd still like to know how this was possible though... Something is definitely holding on to nsHelperAppDlg.xul - as well as causing the shutdown crash it's also triggering local store assertions (trying to flush the local store after the profile has been shut down).
fine, but necko should call OnStopRequest at some point, which releases the reference to mWebProgressListener... I suppose it's possible that this happened before the user chose an action for the file. Urg, wish I remembered this code better...
Comment on attachment 241802 [details] [diff] [review] give nsDownload an owning ref to nsDownloadManager preliminary talkback data suggests this fixed the crash on trunk.
Comment on attachment 241802 [details] [diff] [review] give nsDownload an owning ref to nsDownloadManager In this case, a=me for 1.1b
I landed the crash fix on the branch. I filed bug 356848 for what's described in comment 33 and following.
Verified FIXED on trunk using Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.9a7pre) Gecko/2007072004 Minefield/3.0a7pre, and my Windows Vista trunk build of Minefield, too.