Bug 338897 may have caused other crash

VERIFIED FIXED

Status

()

Core
Event Handling
--
blocker
VERIFIED FIXED
11 years ago
11 years ago

People

(Reporter: smaug, Assigned: smaug)

Tracking

(Depends on: 1 bug, {regression, verified1.8.0.5, verified1.8.1})

1.8 Branch
x86
Linux
regression, verified1.8.0.5, verified1.8.1
Points:
---
Bug Flags:
blocking1.8.1 +
blocking1.8.0.5 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

11 years ago
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)
(Assignee)

Updated

11 years ago
Flags: blocking1.8.0.5?
Keywords: regression

Comment 1

11 years ago
steps to reproduce:

1. Install Spider 
<http://bclary.com/projects/spider/spider/spider.xpi>

2.
<./firefox -chrome "chrome://spider/content/spider.xul?url%3Dhttp%253A%252F%252Ftest.bclary.com%252Ftests%252Fmozilla.org%252Fjs%252Fjs-test-driver-standards.html%253Ftest%253Decma%252FArray%252F15.4-2.js%253Blanguage%253Dtype%253Btext%252Fjavascript%26depth%3D0%26timeout%3D120%26waittime%3D5%26hooksignal%3Don%26autostart%3Don%26autoquit%3Don%26javascripterrors%3Don%26javascriptwarnings%3Don%26csserrors%3Don%26scripturl%3Dhttp%3A%2F%2Ftest.bclary.com%2Ftests%2Fmozilla.org%2Fjs%2Fuserhookeach-js.js">

(Assignee)

Comment 2

11 years ago
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...)
Attachment #226554 - Flags: review?(roc)
(Assignee)

Comment 3

11 years ago
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.
Attachment #226554 - Attachment is obsolete: true
Attachment #226566 - Flags: review?(roc)
Attachment #226554 - Flags: review?(roc)
Attachment #226566 - Flags: superreview+
Attachment #226566 - Flags: review?(roc)
Attachment #226566 - Flags: review+

Comment 4

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

Comment 5

11 years ago
(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. 

Comment 6

11 years ago
I did not crash in linux after applying the second patch, doing make in obj-dbg/widgets and running the example test case.
(Assignee)

Comment 7

11 years ago
This gets called after appService.quit has been called already once.
chrome/content/spider/spider.js:919:    appService.quit(forceQuit);
(Assignee)

Comment 8

11 years ago
(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.

Comment 9

11 years ago
(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?

(Assignee)

Comment 10

11 years ago
toolkit is using eAttemptQuit, spider eForceQuit.
(Assignee)

Comment 11

11 years ago
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.
(Assignee)

Comment 12

11 years ago
Comment on attachment 226566 [details] [diff] [review]
possible patch, even simpler

But still asking approval for 1.8.0
Attachment #226566 - Flags: approval1.8.0.5?
(Assignee)

Updated

11 years ago
Attachment #226566 - Flags: approval1.8.1?
(Assignee)

Updated

11 years ago
Depends on: 342414
(Assignee)

Comment 13

11 years ago
Checked in to trunk
No longer depends on: 342414
(Assignee)

Updated

11 years ago
Depends on: 342414

Comment 14

11 years ago
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?

Updated

11 years ago
Flags: blocking1.8.1?
(Assignee)

Updated

11 years ago
Severity: normal → blocker
(Assignee)

Comment 15

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


Updated

11 years ago
Flags: blocking1.8.1? → blocking1.8.1+
Comment on attachment 226566 [details] [diff] [review]
possible patch, even simpler

approved for 1.8.0 branch, a=dveditz for drivers
Attachment #226566 - Flags: approval1.8.0.5? → approval1.8.0.5+
Flags: blocking1.8.0.5? → blocking1.8.0.5+
Need a non-nobody owner to get this in, and it's Olli's patch: You're the lucky guy! :-)
Assignee: events → Olli.Pettay
(Assignee)

Updated

11 years ago
Keywords: fixed1.8.0.5

Updated

11 years ago
Attachment #226566 - Flags: approval1.8.1? → approval1.8.1+
(Assignee)

Comment 18

11 years ago
Checked in to branches.
bc, I hope you can now run the spider with better success.
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Keywords: fixed1.8.1
Resolution: --- → FIXED
bc, or smaug please verify, thanks

Comment 20

11 years ago
wfm with ff2b2 1.8.0.7, 1.8, 1.9 windows
verified fixed.
Status: RESOLVED → VERIFIED
Keywords: fixed1.8.0.5, fixed1.8.1 → verified1.8.0.5, verified1.8.1
You need to log in before you can comment on or make changes to this bug.