Josh Aas Reporter
	Description • 18 years ago

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).

	Mark Mentovai
	Comment 1 • 18 years ago

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.

	Mark Mentovai
	Comment 2 • 18 years ago

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.

	Hiro
	Comment 3 • 18 years ago

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

	Håkan Waara
	Comment 4 • 18 years ago

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?

	Håkan Waara
	Comment 5 • 18 years ago

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

	Håkan Waara
	Comment 6 • 18 years ago

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

	Jesse Ruderman
	Comment 7 • 18 years ago

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.

	Håkan Waara
	Comment 8 • 18 years ago

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?

	Jesse Ruderman
	Comment 9 • 18 years ago

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

	Håkan Waara
	Comment 10 • 18 years ago

(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.

	Jesse Ruderman
	Comment 13 • 18 years ago

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

	Jesse Ruderman
	Comment 14 • 18 years ago

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.

	Håkan Waara
	Comment 15 • 18 years ago

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

	Tony Chang (Google) Assignee
	Comment 16 • 18 years ago

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.

	Darin Fisher
	Comment 17 • 18 years ago

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.

	Tony Chang (Google) Assignee
	Comment 19 • 18 years ago

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

	Benjamin Smedberg
	Comment 20 • 18 years ago

Can this be unit-tested in xpcshell?

	Josh Aas Reporter
	Comment 21 • 18 years ago

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.

	Mark Mentovai
	Comment 22 • 18 years ago

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.

	Bob Clary [:bc] (inactive)
	Comment 23 • 18 years ago

(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.

	Josh Aas Reporter
	Comment 24 • 18 years ago

landed the appshell fix