343033 - 5-10 second delay or hang or crash when quitting Cocoa Firefox

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect)

Description

[Josh Aas]

When quitting Cocoa Firefox, say via command-q, there is a 5-10 second delay before the app actually quits (time varies from machine to machine, but its always way too long).

Comment 1

[Mark Mentovai]

This might be someone blocking on [NSApplication nextEventMatchingMask] after everything's supposed to be shutting down.  Posting a null event in nsAppShell::WillTerminate might fix this.

Comment 2

[Mark Mentovai]

We're not even getting WillTerminate soon enough when the quit comes from Command-Q, which is part of the problem.  "Native" shutdowns are OK.

Comment 3

[Hiro]

Do neither bug341850 nor bug341384 relate?
(In Firefox, time of these problems hangs in shutdown.)

Comment 4

[Håkan Waara]

I'm seeing this every day.

I wonder if it's relevant, but there's often a note that the whole RDF service is leaked. Is that normal?

Comment 5

[Håkan Waara]

I was trying to quit. Switched to another app, and then saw that Minefield was still in the Dock. So, when I realized that I was (again) experiencing this bug, I looked in the console. It was sitting at the first line in this excerpt, and then quickly managed to quit.

From the console:

nsPluginHostImpl::Observe "xpcom-shutdown"
WARNING: nsExceptionService ignoring thread destruction after shutdown: file /Users/hakan/Documents/Programmering/firefox/mozilla/xpcom/base/nsExceptionService.cpp, line 191
nsPluginHostImpl dtor
WARNING: NS_ENSURE_TRUE(NS_SUCCEEDED(rv)) failed: file /Users/hakan/Documents/Programmering/firefox/mozilla/toolkit/components/history/src/nsGlobalHistory.cpp, line 2673
WARNING: NS_ENSURE_TRUE(NS_SUCCEEDED(OpenDB())) failed: file /Users/hakan/Documents/Programmering/firefox/mozilla/toolkit/components/history/src/nsGlobalHistory.cpp, line 1269
--DOMWINDOW == 1
--DOMWINDOW == 0
+++ JavaScript debugging hooks removed.
JS engine warning: 775 atoms remain after destroying the JSRuntime.
                   These atoms may point to freed memory. Things reachable
                   through them have not been finalized.
nsStringStats
 => mAllocCount:          22037
 => mReallocCount:         2341
 => mFreeCount:           21832  --  LEAKED 205 !!!
 => mShareCount:          15311
 => mAdoptCount:           1937
 => mAdoptFreeCount:       1937

Comment 6

[Håkan Waara]

Attachment: Crash when quitting

I also get a crash if I try to look in the menu when I'm waiting (sometimes as long as 20 secs) for Firefox to quit. Steps to crash on quit:

1. Cmd-Q to quit
2. Click in the menu

The crash is inside the menu handling code

Comment 7

[Jesse Ruderman]

Since Cocoa widgets were enabled, I've seen each of the following when I try to quit with Cmd+Q or programatically:

* 5-second delay.
* Indefinite hang, but if I click the dock icon, it finally quits.
* Crash.

This makes it very hard for me to do automated testing on Gecko to find security holes, regressions that cause crashes, etc.

Comment 8

[Håkan Waara]

Wow, I can't believe I missed this. Look closely at the stack in attachment 241070 [details]. Excerpt:

...
23  libtoolkitcomps.dylib 0x089a0ef4 nsUrlClassifierDBService::Shutdown() + 508 (nsUrlClassifierDBService.cpp:1003)
24  libtoolkitcomps.dylib 0x089a0fb8 nsUrlClassifierDBService::Observe(nsISupports*, char const*, unsigned short const*) + 92 (nsUrlClassifierDBService.cpp:978)
...

Basically, I think the phishing service is dead-locked, so we can't quit and our menubar is around forever. When you try to use it, everything crashes.

See bug 353962 which is also about mac build (firefox 2) hanging due to a possible dead-lock in the phishing code. Might be the same cause?

Comment 9

[Jesse Ruderman]

Hmm, my crash doesn't have any nsUrlClassifierDBService on the stack and might be a different bug.

Comment 10

[Håkan Waara]

(In reply to comment #9)
> Hmm, my crash doesn't have any nsUrlClassifierDBService on the stack and might
> be a different bug.

If you have a different stack, please attach it here.

Comment 11

[Jesse Ruderman]

Attachment: Jesse's testcase, quit-browser.html (requires privs)

Comment 12

[Jesse Ruderman]

Attachment: Jesse's stack trace, with [@ nsCocoaWindow::GetAttention] at the top

Comment 13

[Jesse Ruderman]

It's looking like my crash is a separate bug, so I filed bug 358607.

Comment 14

[Jesse Ruderman]

On the other hand, I need both the crash and the hang fixed in order to really use goQuitApplication as part of automated testing :(

Philor pointed out that I can build without Cocoa for now using "--enable-default-toolkit=mac", so I'm doing that.

Comment 15

[Håkan Waara]

*** Bug 344093 has been marked as a duplicate of this bug. ***

Comment 16

[Tony Chang (Google)]

Attachment: cancel background download as soon as user quits

It looks like if the download is still pending, it can delay the shutdown for a few seconds (although eventually the main thread goes away and the http connection is canceled because it has nowhere to post an event).  This kills the download faster.

Comment 17

[Darin Fisher]

Comment on attachment 245926 [details] [diff] [review]
cancel background download as soon as user quits

nits: mChannel is a nsIRequest, so QI is not necessary.

perhaps it would be worth it to define a "quit-application" macro some place so you don't have to hard-code it twice.

Comment 18

[Tony Chang (Google)]

Attachment: v2: cancel background download with feedback

Comment 19

[Tony Chang (Google)]

On trunk

Checking in nsUrlClassifierDBService.cpp;
/cvsroot/mozilla/toolkit/components/url-classifier/src/nsUrlClassifierDBService.cpp,v  <--  nsUrlClassifierDBService.cpp
new revision: 1.20; previous revision: 1.19
done
Checking in nsUrlClassifierStreamUpdater.cpp;
/cvsroot/mozilla/toolkit/components/url-classifier/src/nsUrlClassifierStreamUpdater.cpp,v  <--  nsUrlClassifierStreamUpdater.cpp
new revision: 1.8; previous revision: 1.7
done
Checking in nsUrlClassifierStreamUpdater.h;
/cvsroot/mozilla/toolkit/components/url-classifier/src/nsUrlClassifierStreamUpdater.h,v  <--  nsUrlClassifierStreamUpdater.h
new revision: 1.4; previous revision: 1.3
done

Comment 20

[Benjamin Smedberg]

Can this be unit-tested in xpcshell?

Comment 21

[Josh Aas]

Attachment: appshell fix v1.0

When I filed this bug I think I was talking about the fact that the app will just hang on shutdown when you quit via gecko, like through a menu command. That is not fixed at this point.

This patch fixes it, though it isn't how we want to do it in the long run. We need to eventually rewrite the appshell so it doesn't have two runloops. I'll back this out if it has Tp or Ts issues.

Comment 22

[Mark Mentovai]

Comment on attachment 250503 [details] [diff] [review]
appshell fix v1.0

As discussed.

The pair of run loops was necessary to avoid crashing when [NSApp run] wasn't on the stack - it is itself a dirty trick.

Comment 23

[Bob Clary [:bc] (inactive)]

(In reply to comment #21)
> Created an attachment (id=250503) [details]
> appshell fix v1.0

This fixes the problem of goQuitApplication not being able to shutdown mac trunk builds.

Comment 24

[Josh Aas]

landed the appshell fix