185360 - uncaught exception calling goQuitApplication() [nsIAppShellService.quit]

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[Nathan Kurz]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.3a) Gecko/20021207 Phoenix/0.5
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.3a) Gecko/20021207 Phoenix/0.5

When I call goQuitApplication() (xpfe/global/resources/content/globalOverlay.js) 

JavaScript error: 
 line 0: uncaught exception: [Exception... "Component returned failure code:
0x80004005 (NS_ERROR_FAILURE) [nsIAppShellService.quit]"  nsresult: "0x80004005
(NS_ERROR_FAILURE)"  location: "JS frame ::
chrome://global/content/globalOverlay.js :: goQuitApplication :: line 38"  data: no]

This caused by line 576 in xpfe/appshell/src/nsAppShellService.cpp.  All windows
have not yet closed, so NS_ERROR_FAILURE is returned.  Either this should be
changed to an NS_SUCCESS, or goQuitApplication() should catch this exception.



Reproducible: Always

Steps to Reproduce:

Comment 1

[Brendan Eich [:brendan]]

danm's most likely to confirm this bug.

/be

Comment 2

[timeless]

i usually get this if i play poorly w/ venkman, i haven't gotten it since my
travels, but i've been poking other parts of venkman instead.

Comment 3

[Karsten Düsterloh]

I can see this with a 2004-01-12 CVS trunk debug build on Win2k; the actual line
has changed, though:

JavaScript error:
 line 0: uncaught exception: [Exception... "Component returned failure code:
0x80004005 (NS_ERROR_FAILURE) [nsIAppShellService.quit]"  nsresult: "0x800040
05 (NS_ERROR_FAILURE)"  location: "JS frame
::chrome://global/content/globalOverlay.js :: goQuitApplication :: line 40" 
data: no]

It comes after
   WEBSHELL+ = 4
but before
   nsPluginHostImpl::Observe "quit-application"

I get this error *only* when closing Mozilla via CTRL-Q and *not* when closing
via the window's upper right [x] button.

Comment 4

[Dan M]

Attachment: appshellservice::quit returns no error under normal circumstances

Yes, an attempt to quit doesn't necessarily succeed immediately even if it does
set into motion things that will make the app quit soon. It's likely the DOM
window will stumble on until the next JS garbage collection cycle. That's
happening here, attempting to quit the app using a command handler.

Still, I could have done this better. This patch makes appshellservice::quit
return no error unless something really has gone awry. It will inaccurately
claim success if, say, you try to quit the app with an open window containing
an unload handler that opens a new window (and popup controls disabled). But no
caller of Quit cares about such hair splitting, so I claim this is fine.

Comment 5

[Karsten Düsterloh]

I applied the patch to my tree and did a clean rebuild of /xpfe, but that didn't
change anything wrt the JS error...

Comment 6

[Dan M]

That's odd. I get an error that looks just like the one you describe in comment
3 if I revert my change*. After reapplying my patch the log is identical except
that error is missing. What are you doing? I'm launching a Windows Seamonkey
debug build and hitting ^Q a moment after the first browser window appears. (And
I get the same results of I first open a second browser, or the mail window.)
NB: you need rebuild only xpfe/appshell/src.

* though, also odd, my webshell count never reaches 4; i get only 3.

Comment 7

[Karsten Düsterloh]

I CVS-updated and rebuild my whole trunk build yesterday with your patch
included and now the exception is gone! :)

(Seems as if just building xpfe did not suffice - but a complete build takes
about 2,5 hours on my slow machine, so I avoid it whenever possible.)

The difference in webshell counting is due to my -mail when starting Seamonkey.
Time to get r/sr, I guess! ;-)

Comment 8

[Martin Mokrejs]

Have similar error when quitting mozilla-1.6 release on Tru64Unix 5.1A:

JavaScript error: 
 line 0: uncaught exception: [Exception... "Component returned failure code:
0x80004005 (NS_ERROR_FAILURE) [nsIAppShellService.quit]"  nsresult: "0x80004005
(NS_ERROR_FAILURE)"  location: "JS frame ::
chrome://global/content/globalOverlay.js :: goQuitApplication :: line 40"  data: no]

GetPrimaryFrameFor() called while FrameManager is being destroyed!
WARNING: NS_ENSURE_TRUE(presContext) failed, file nsGenericHTMLElement.cpp, line
4303
WARNING: NS_ENSURE_TRUE(presContext) failed, file nsGenericHTMLElement.cpp, line
4303
nsPluginHostImpl::Observe "quit-application"
WARNING: requested removal of nonexistent window
, file nsWindowWatcher.cpp, line 945
WEBSHELL- = 2
WEBSHELL- = 1
WEBSHELL- = 0
nsPluginHostImpl::Observe "xpcom-shutdown"
WARNING: nsExceptionService ignoring thread destruction after shutdown, file
nsExceptionService.cpp, line 189
+++ JavaScript debugging hooks removed.
nsPluginHostImpl dtor
GC Cache:
        hits:  869  284  181   57   15    1    4    0    1   10
        hits: 1422, misses: 352, hit percent: 80.157837%

Comment 9

[Martin Mokrejs]

I tried latest cvs (Mozilla/5.0 (X11; U; OSF1 alpha; en-US; rv:1.7a)
Gecko/20040201) on Tru64Unix and am still getting this error:

JavaScript error: 
 line 0: uncaught exception: [Exception... "Component returned failure code:
0x80004005 (NS_ERROR_FAILURE) [nsIAppShellService.quit]"  nsresult: "0x80004005
(NS_ERROR_FAILURE)"  location: "JS frame ::
chrome://global/content/globalOverlay.js :: goQuitApplication :: line 40"  data: no]

GetPrimaryFrameFor() called while FrameManager is being destroyed!
--WEBSHELL == 2
nsPluginHostImpl::Observe "quit-application"
WARNING: requested removal of nonexistent window
, file nsWindowWatcher.cpp, line 945
--WEBSHELL == 1
--WEBSHELL == 0
nsPluginHostImpl::Observe "xpcom-shutdown"
--DOMWINDOW == 2
WARNING: nsExceptionService ignoring thread destruction after shutdown, file
nsExceptionService.cpp, line 191
--DOMWINDOW == 1
--DOMWINDOW == 0
+++ JavaScript debugging hooks removed.
nsPluginHostImpl dtor
GC Cache:
        hits: 1254  290  184  579   50   37   34   59   22    9
        hits: 2518, misses: 825, hit percent: 75.321564%

Comment 10

[Martin Mokrejs]

Hmm, I thought you patch is already in public cvs. After manully patching my cvs
copy I can confirm the uncaught exception messages disappeared.

Comment 11

[jag (Peter Annema)]

Comment on attachment 139852 [details] [diff] [review]
appshellservice::quit returns no error under normal circumstances

So you changed the check from "do I still have any DOM windows?" to "do I still
have any non-closed DOM windows?". Works for me.

If we ever start doing things like unloading critical DLLs that won't
automatically get re-loaded when a popup opens from onunload, then we can worry
about making the check here more pedantic.

sr=jag

Comment 12

[[e-mail]]

Comment on attachment 139852 [details] [diff] [review]
appshellservice::quit returns no error under normal circumstances

>-          if (NS_SUCCEEDED(windowEnumerator->HasMoreElements(&more)) && more) {
>+          while (windowEnumerator->HasMoreElements(&more), more) {
I supposed this change is safe.

Comment 13

[Dan M]

Overall this change is little less benign than a spelling change. About that
particular line; I've noticed I get hairy palms from slavishly heeding the
&#%#)*# error return under all circumstances. I find it no more dangerous but
considerably more aesthetically pleasing in this form, so thanks for seeing it
on its way. Checked in to 1.7b.