338249 - Embedding applications exit/quit/vanish prematurely due to stopping shared NSApplication

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect)

Description

[philippe (part-time)]

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en; rv:1.9a1) Gecko/20060510 Camino/1.2+
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en; rv:1.9a1) Gecko/20060510 Camino/1.2+

With the patches in bug 337824, when uxing command tab to switch between apps, the browser quits without warning. From the console.log

2006-05-17 11:24:55.759 Camino[15526] *** _NSAutoreleaseNoPool(): Object 0x1ed81d0 of class NSCFArray autoreleased with no pool in place - just leaking
2006-05-17 11:24:55.760 Camino[15526] *** _NSAutoreleaseNoPool(): Object 0x640b7b0 of class NSCFString autoreleased with no pool in place - just leaking
2006-05-17 11:24:55.760 Camino[15526] *** _NSAutoreleaseNoPool(): Object 0x1ea5550 of class NSCFString autoreleased with no pool in place - just leaking
2006-05-17 11:24:55.760 Camino[15526] *** _NSAutoreleaseNoPool(): Object 0x64d9a30 of class NSCFSet autoreleased with no pool in place - just leaking
2006-05-17 11:24:55.761 Camino[15526] *** _NSAutoreleaseNoPool(): Object 0x1e1cb00 of class NSCFNumber autoreleased with no pool in place - just leaking
2006-05-17 11:24:55.761 Camino[15526] *** _NSAutoreleaseNoPool(): Object 0x6443130 of class NSCFSet autoreleased with no pool in place - just leaking
2006-05-17 11:24:55.761 Camino[15526] *** _NSAutoreleaseNoPool(): Object 0x64e0090 of class NSCFSet autoreleased with no pool in place - just leaking
2006-05-17 11:24:55.761 Camino[15526] *** _NSAutoreleaseNoPool(): Object 0x641b3a0 of class BrowserTabViewItem autoreleased with no pool in place - just leaking
2006-05-17 11:24:55.768 Camino[15526] *** _NSAutoreleaseNoPool(): Object 0x67ca1f0 of class NativeScrollbarView autoreleased with no pool in place - just leaking
2006-05-17 11:24:55.768 Camino[15526] *** _NSAutoreleaseNoPool(): Object 0x67c9070 of class NativeScrollbarView autoreleased with no pool in place - just leaking
2006-05-17 11:24:55.769 Camino[15526] *** _NSAutoreleaseNoPool(): Object 0x67c93c0 of class ChildView autoreleased with no pool in place - just leaking
2006-05-17 11:24:55.769 Camino[15526] *** _NSAutoreleaseNoPool(): Object 0x6704c60 of class ChildView autoreleased with no pool in place - just leaking
2006-05-17 11:24:55.770 Camino[15526] *** _NSAutoreleaseNoPool(): Object 0x645bbf0 of class ChildView autoreleased with no pool in place - just leaking
2006-05-17 11:24:55.771 Camino[15526] *** _NSAutoreleaseNoPool(): Object 0x1e97bf0 of class BrowserTabViewItem autoreleased with no pool in place - just leaking
2006-05-17 11:24:55.778 Camino[15526] *** _NSAutoreleaseNoPool(): Object 0x1eb0fc0 of class NativeScrollbarView autoreleased with no pool in place - just leaking
2006-05-17 11:24:55.779 Camino[15526] *** _NSAutoreleaseNoPool(): Object 0x67f68f0 of class NativeScrollbarView autoreleased with no pool in place - just leaking
2006-05-17 11:24:55.785 Camino[15526] *** _NSAutoreleaseNoPool(): Object 0x6485d60 of class ChildView autoreleased with no pool in place - just leaking
2006-05-17 11:24:55.786 Camino[15526] *** _NSAutoreleaseNoPool(): Object 0x1ea46a0 of class ChildView autoreleased with no pool in place - just leaking
2006-05-17 11:24:55.795 Camino[15526] *** _NSAutoreleaseNoPool(): Object 0x6703bd0 of class ChildView autoreleased with no pool in place - just leaking
2006-05-17 11:24:55.795 Camino[15526] *** _NSAutoreleaseNoPool(): Object 0x1e7ac40 of class NSIdEnumerator autoreleased with no pool in place - just leaking
2006-05-17 11:24:55.796 Camino[15526] *** _NSAutoreleaseNoPool(): Object 0x69d84c0 of class TabButtonCell autoreleased with no pool in place - just leaking
2006-05-17 11:24:55.796 Camino[15526] *** _NSAutoreleaseNoPool(): Object 0x1ef8620 of class BrowserWindowController autoreleased with no pool in place - just leaking
2006-05-17 11:24:55.796 Camino[15526] *** _NSAutoreleaseNoPool(): Object 0x645c0e0 of class TopLevelWindowData autoreleased with no pool in place - just leaking
2006-05-17 11:24:55.797 Camino[15526] *** _NSAutoreleaseNoPool(): Object 0x643c070 of class NSCFString autoreleased with no pool in place - just leaking

(from bug 337824 comment 20

Reproducible: Always

Comment 1

[Mark Mentovai]

Hmm.  OK.  When I first looked at this, I was able to crash by quitting Camino using Command-Q, producing a similar stream of warnings about there not being an autorelease pool.  That's where my diagnosis of dispatching native events after [NSApp terminate:] came from.  Now that I read your report more closely, I see that's not the exact bug you're complaining of, unless your finger slipped and hit Q instead of tab.

Can you (or anyone else) confirm this?  I'm not seeing the app crash or quit at all when I command-tab to it, from it, or through it, but I do see a problem with command-Q.

When this happens, does it crash?  A CrashReporter log or gdb trace would be helpful.

Comment 2

[Mark Mentovai]

Attachment: Patch for command-Q problem

This fixes the command-Q problem I am seeing, which may not be the bug as reported here.

Comment 3

[philippe (part-time)]

My finger may have slipped once, but not more :-).

I tried to repro the problem multiple times, with the same results in the console.log.
First with my own build, built shortly after darin's patches (bug 337824) relanded, next with tinderbox builds (2006051617 (1.2+), 2006051619 (1.2+)).
I've even been able to see Camino quit on me (vanish might be a better word) while filling a textarea on some forums.
The command tab acction is the most easy to reproduce, though.
And with a fresh profile.

Comment 4

[philippe (part-time)]

Another way to reproduce this:
Open a page and let it rest there for a moment (like reading an article).
scroll down with the space bar --> poof, quit
(I had it happening with this page: http://www.lemonde.fr/)

Version 2006051619 (1.2+)

Again, same output to the console log, no crash log.

Comment 5

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

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

Comment 6

[Mark Mentovai]

I suspect that the random quits are being caused by calling [NSApp stop] in widget/src/cocoa/nsAppShell.mm.

Comment 7

[Håkan Waara]

Mark, any way we can test your hypothesis?

Comment 8

[Mark Mentovai]

Attachment: Don't use [NSApp run] or [NSApp stop:]

Try this.

Comment 9

[Håkan Waara]

I've been using this patch for a few hours, and it still hasn't crashed since, whereas it would crash every 5 minutes before.

Comment 10

[philippe (part-time)]

With this patch applied: uptime: 3.25min. :-)

But then I quit the browser: --> crash
Crash log coming. Console log shows this again:

2006-05-21 12:21:43.157 Camino[15512] *** _NSAutoreleaseNoPool(): Object 0x61e4490 of class NSIdEnumerator autoreleased with no pool in place - just leaking
2006-05-21 12:21:43.158 Camino[15512] *** _NSAutoreleaseNoPool(): Object 0x8c5a4f0 of class TabButtonCell autoreleased with no pool in place - just leaking
2006-05-21 12:21:43.158 Camino[15512] *** _NSAutoreleaseNoPool(): Object 0x1ea5ce0 of class BrowserWindowController autoreleased with no pool in place - just leaking
2006-05-21 12:21:43.158 Camino[15512] *** _NSAutoreleaseNoPool(): Object 0x64ce840 of class TopLevelWindowData autoreleased with no pool in place - just leaking
2006-05-21 12:21:43.159 Camino[15512] *** _NSAutoreleaseNoPool(): Object 0x640e620 of class NSCFString autoreleased with no pool in place - just leaking

---
I didn't experience bug 337550 comment #24.

Comment 11

[philippe (part-time)]

Attachment: crash log, see comment #10

Crash on quit (command Q) with patch in comment #8 applied

Comment 12

[philippe (part-time)]

(In reply to comment #10)
> With this patch applied: uptime: 3.25min. :-)

should be 3.25 _hours_

Comment 13

[Mark Mentovai]

Comment 10 /et. seq./ is forked into bug 338728 in the interests of "one bug per report."  It's what I was referring to in comment 1, and what the first patch on this bug, attachment 222334 [details] [diff] [review], fixes.  Other patches attached to this bug don't take the command-Q problem into account.

Comment 14

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

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

Comment 15

[philippe (part-time)]

(In reply to comment #13)
> Comment 10 /et. seq./ is forked into bug 338728 in the interests of "one bug
> per report."  It's what I was referring to in comment 1, and what the first
> patch on this bug, attachment 222334 [details] [diff] [review] [edit], fixes.  Other patches attached to this
> bug don't take the command-Q problem into account.
> 

Ah, my misunderstanding there.
That said, attachment (id=222628) works wonderfully well. Absolutely no problems since I started using it.

Comment 16

[Mark Mentovai]

Comment on attachment 222628 [details] [diff] [review]
Don't use [NSApp run] or [NSApp stop:]

This is the Carbon version of the Cocoa patch in bug 338249.  Here's the story:

Because we need to support embedding and plugins and all sorts of other things that expect the world to behave a certain way, we really can't get away with ::RunApplicationEventLoop()/::QuitApplicationEventLoop() and [NSApp run]/[NSApp stop:] as I had initially hoped.  This means that there's a chance of being bitten in the butt if we don't run our event loop the same way the system would, or if the system implementations of the event loops start doing something that we don't do in the future.  That's a risk I was hoping to avoid, but it's one we're going to have to take.

For Carbon at least, there are some well-documented things that you need to provide for when switching from ::RunApplicationEventLoop() to a self-run ::ReceiveNextEvent loop, documented at http://developer.apple.com/documentation/Carbon/Conceptual/Carbon_Event_Manager/Tasks/chapter_3_section_12.html .  For one-time setup that RAEL would do, such as installing certain handlers, Apple suggests running the entire app on an event dispatched by RAEL, but because we just migrated from the WNE world, these things are already accounted for in our code.  We just need to hope that there aren't any surprises.

As for Cocoa, the loop's internals aren't really documented, but I've done what I can to match what the system does based on what I know of it and based on the GNUstep implementation.

Comment 17

[Mark Mentovai]

P.S. You'll note that although it's no longer necessary when running the loop ourselves, I've still got ProcessNextNativeEvent running in its own tight loop when aMayWait is PR_TRUE.  I've done this because I've heard that we still might not be leaving the native loop to process Gecko events in all cases where we should (bug 337550 comment 24), and I'd like to be able to catch those before making PNNE behave like other platforms (with the side-effect of masking many cases of dysfunctional ScheduleNativeEventCallback behavior).

Comment 18

[Darin Fisher]

Comment on attachment 222628 [details] [diff] [review]
Don't use [NSApp run] or [NSApp stop:]

sr=darin w/ same comments from bug 339051.  Please make PNNE return true if any event was processed and false if no events were processed.

Comment 19

[Mark Mentovai]

Followup bug is bug 339353.

Comment 20

[Mark Mentovai]

Checked in on trunk with review comments addressed.

Comment 21

[Mark Mentovai]

Comment on attachment 222334 [details] [diff] [review]
Patch for command-Q problem

Obsolete, go to bug 338728 for continuation.