396833 - "WARNING: nsAppShell::Exit() called redundantly" on shutdown

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect)

Description

[Jesse Ruderman]

Steps to reproduce:
1. Start a debug build of Firefox (from the command line).
2. Click on the Firefox window to focus it.
3. Cmd+Q.

Result:

###!!! ASSERTION: nsAppShell::Exit() called redundantly: '!mTerminated', file /Users/jruderman/trunk/mozilla/widget/src/cocoa/nsAppShell.mm, line 490

I don't know why this didn't turn Tinderboxen orange.  Does this not happen there, or do they use a different method to quit?

Comment 1

[Steven Michaud [:smichaud] (Retired)]

This is an old problem.  It's just that my patch has started
complaining about it.

Here's the source-code comment above the assertion:

// This method is currently called more than once -- from (according to
// mento) an nsAppExitEvent dispatched by nsAppStartup::Quit() and from an
// XPCOM shutdown notification that nsBaseAppShell has registered to
// receive.  So we need to ensure that multiple calls won't break anything.
// But we should also complain about it (since it isn't quite kosher).

Comment 2

[Josh Aas]

Steven - what is your recommendation about making this a blocker bug? Is this just a warning that we can ignore or silence or something we need to actually fix?

Comment 3

[Steven Michaud [:smichaud] (Retired)]

It's an old, old bug (I saw it when I first started working on the
appshell back in April).  I don't believe it has any user-visible
consequences (otherwise somebody else would have discovered it, too).
It may be a sign of something else wrong that's more serious.  But for
itself it's really not worth blocking on.

I actually now regret putting in the assertion.  Should I get rid of
it?

Comment 4

[Jesse Ruderman]

Yes, please.  A bug report should be enough of a reminder that the bug exists.

Comment 5

[Josh Aas]

Lets get rid of the assertion and leave a comment.

Comment 6

[Jeff Walden [:Waldo]]

This assertion is the only assertion firing on the Mac debug+leak tinderbox, and it is the only bug preventing assertions from being fatal on that box.

Comment 7

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

You could also change the assertion to a warning.

Comment 8

[Jeff Walden [:Waldo]]

Attachment: Convert to a warning

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

BTW you can write this more simply using NS_WARN_IF_FALSE

Comment 10

[Jeff Walden [:Waldo]]

Ah, didn't know about that macro, and didn't read the second comment until after I committed.  In any case, tho, I think this is actually a little more readable, because to use that you need a double-negative.

I'll let Jesse decide whether to mark this bug fixed or to morph it to not hit the warning, as it's blocking his console-spam bug (and we didn't get rid of the console spam, only changed it).

Comment 11

[Mike Connor [:mconnor]]

Jesse, please reopen or file a new bug if there's more to do here.

Comment 12

[Jesse Ruderman]

Ok, I filed bug 402497 on fixing the warning.