According to bc, there has been a lot more crashes lately in 1.8.0 and 1.8 (Linux only). I can't reproduce the crash (even with bc's exact instructions, which required one to install an extension and run some specific chrome:// url...), but without Bug 338897 I don't see a gtk assertion which is there otherwise (Gecko:5685): Gtk-CRITICAL **: gtk_widget_destroy: assertion `GTK_IS_WIDGET (widget)' failed Stack from bc: Stack: .L30 (/work/mozilla/builds/ff/1.5.0/mozilla/obj-dbg/toolkit/profile/src/nsProfileLock.cpp:210) UNKNOWN (/lib/tls/libpthread.so.0) gtk_widget_destroy+0x00000055 (/usr/lib/libgtk-x11-2.0.so.0) nsDragService::Observe(nsISupports*, char const*, unsigned short const*) (/work/mozilla/builds/ff/1.5.0/mozilla/widget/src/gtk2/nsDragService.cpp:136) nsObserverService::NotifyObservers(nsISupports*, char const*, unsigned short const*) (/work/mozilla/builds/ff/1.5.0/mozilla/xpcom/ds/nsObserverService.cpp:233) nsAppStartup::Quit(unsigned int) (/work/mozilla/builds/ff/1.5.0/mozilla/toolkit/components/startup/src/nsAppStartup.cpp:284) nsAppStartup::Observe(nsISupports*, char const*, unsigned short const*) (/work/mozilla/builds/ff/1.5.0/mozilla/toolkit/components/startup/src/nsAppStartup.cpp:522) nsObserverService::NotifyObservers(nsISupports*, char const*, unsigned short const*) (/work/mozilla/builds/ff/1.5.0/mozilla/xpcom/ds/nsObserverService.cpp:233) nsXULWindow::Destroy() (/work/mozilla/builds/ff/1.5.0/mozilla/xpfe/appshell/src/nsXULWindow.cpp:551) nsWebShellWindow::Destroy() (/work/mozilla/builds/ff/1.5.0/mozilla/xpfe/appshell/src/nsWebShellWindow.cpp:850) nsChromeTreeOwner::Destroy() (/work/mozilla/builds/ff/1.5.0/mozilla/xpfe/appshell/src/nsChromeTreeOwner.cpp:353) nsGlobalWindow::ReallyCloseWindow() (/work/mozilla/builds/ff/1.5.0/mozilla/dom/src/base/nsGlobalWindow.cpp:4689) nsCloseEvent::HandleEvent() (/work/mozilla/builds/ff/1.5.0/mozilla/dom/src/base/nsGlobalWindow.cpp:4454) HandleCloseEvent (/work/mozilla/builds/ff/1.5.0/mozilla/dom/src/base/nsGlobalWindow.cpp:4464) PL_HandleEvent (/work/mozilla/builds/ff/1.5.0/mozilla/xpcom/threads/plevent.c:688) PL_ProcessPendingEvents (/work/mozilla/builds/ff/1.5.0/mozilla/xpcom/threads/plevent.c:623) nsEventQueueImpl::ProcessPendingEvents() (/work/mozilla/builds/ff/1.5.0/mozilla/xpcom/threads/nsEventQueue.cpp:417) event_processor_callback (/work/mozilla/builds/ff/1.5.0/mozilla/widget/src/gtk2/nsAppShell.cpp:67) UNKNOWN (/usr/lib/libglib-2.0.so.0) g_main_context_dispatch+0x000001CB (/usr/lib/libglib-2.0.so.0) UNKNOWN (/usr/lib/libglib-2.0.so.0) g_main_loop_run+0x000001A9 (/usr/lib/libglib-2.0.so.0) gtk_main+0x000000B2 (/usr/lib/libgtk-x11-2.0.so.0) nsAppShell::Run() (/work/mozilla/builds/ff/1.5.0/mozilla/widget/src/gtk2/nsAppShell.cpp:141) nsAppStartup::Run() (/work/mozilla/builds/ff/1.5.0/mozilla/toolkit/components/startup/src/nsAppStartup.cpp:150) XRE_main (/work/mozilla/builds/ff/1.5.0/mozilla/toolkit/xre/nsAppRunner.cpp:2374) main (/work/mozilla/builds/ff/1.5.0/mozilla/browser/app/nsBrowserApp.cpp:61) __libc_start_main+0x000000D3 (/lib/tls/libc.so.6)
Created attachment 226554 [details] [diff] [review] possible patch When I use --g-fatal-warnings I can see the crash. This patch helps for that, though I'm not at all familiar with gtk code. (But right now way too tired to decide what I'd like to do with this crash - is the patch safe enough for branch or should Bug 338897 be backed out...)
Created attachment 226566 [details] [diff] [review] possible patch, even simpler Now someone should just tell why "quit-application" is called twice with the test case.
bc: Can you answer smaug's question about why we call quit twice? Anyway to test this fix with your test automation to see if the crashes go away? If not, we will need to approve this patch for the 1.8.0 branch and get it checked in so you can retest.
(In reply to comment #4) > bc: Can you answer smaug's question about why we call quit twice? No. It isn't obvious to me why quit-application is sent to the observers twice unless the xpfe code is notifying as well as the toolkit code. > Anyway to test this fix with your test automation to see if the crashes go away? I can try the particular test mentioned in this bug. I don't know if that would be a sufficient test but I guess it would be better than nothing.
I did not crash in linux after applying the second patch, doing make in obj-dbg/widgets and running the example test case.
This gets called after appService.quit has been called already once. chrome/content/spider/spider.js:919: appService.quit(forceQuit);
(In reply to comment #7) > This gets called after appService.quit has been called already once. > chrome/content/spider/spider.js:919: appService.quit(forceQuit); > er, or otherway round.
(In reply to comment #8) This was modeled after <http://lxr.mozilla.org/mozilla1.8.0/source/toolkit/content/globalOverlay.js#47>. Maybe the cause is a name collision with Spider's version of goQuitApplication and the toolkit version. Can you tell who is calling whom?
toolkit is using eAttemptQuit, spider eForceQuit.
And there is this comment: http://lxr.mozilla.org/seamonkey/source/toolkit/components/startup/src/nsAppStartup.cpp#194 I tried spider with eAttemptQuit and that way it seemed to work ok even without the crash fix patch.
Comment on attachment 226566 [details] [diff] [review] possible patch, even simpler But still asking approval for 1.8.0
Checked in to trunk
I've seen that comment and it is BS. I have to use eForceQuit or Firefox will not terminate on Mac OS X. So, toolkit is calling its goQuitApplication when Spider completes iterating over and closing all windows?
(In reply to comment #14) > So, toolkit is calling its goQuitApplication when > Spider completes iterating over and closing all windows? > First Spider calls appService.quit(eForceQuit) ( which doesn't close any windows). Then the last window is closed which causes "xul-window-destroyed" message, which is handled in nsAppStartup::Observe by calling Quit(eConsiderQuit).
Comment on attachment 226566 [details] [diff] [review] possible patch, even simpler approved for 1.8.0 branch, a=dveditz for drivers
Need a non-nobody owner to get this in, and it's Olli's patch: You're the lucky guy! :-)
Checked in to branches. bc, I hope you can now run the spider with better success.
bc, or smaug please verify, thanks
wfm with ff2b2 22.214.171.124, 1.8, 1.9 windows verified fixed.