337824 - Profile Manager completely horked

Status: RESOLVED FIXED

(Toolkit :: Startup and Profile System, defect, P1)

Description

[Patrick]

Using Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a1) Gecko/20060512 Minefield/3.0a1

starting Minefield from the Mac command line via
   Minefield.app/Contents/MacOS $ ./firefox-bin -p
shows the profile manager, however, the window that appears is not functional: 
- one is not able to select any of the other profiles
- pressing START MINEFIELD with the default selected profile results in a hang
- buttons for DELETE and CREATE work only after pressing them twice
- pressing CANCEL during the profile creation ends in a loop of two or three dialogs to start creating profiles, with the final dialog being totally blank
- pressing EXIT keeps Minefield active [without windows]
- keyboard navigations seems broken

This severely limits testing possibilities and thus marking this as a Blocker ["Blocks development and/or testing work"]

Comment 1

[Patrick]

Only on Trunk, wfm with 
Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8.1a2) Gecko/20060512 BonEcho/2.0a2

Comment 2

[Patrick]

Regression window:
WORKS : Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a1) Gecko/20060510 Minefield/3.0a1

BROKEN : Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a1) Gecko/20060511 Minefield/3.0a1

Comment 3

[Ria Klaassen (not reading all bugmail)]

WFM with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060512 Minefield/3.0a1, although I've not tested the keyboard navigations.

Comment 4

[Patrick]

darin, could your checkins during the period mentioned in comment 2 have caused this?

Comment 5

[:Gavin Sharp [email: [e-mail]]]

Yes, this is a regression from Darin's thread changes.
See http://pastebin.ca/55487 (and bug 337550/bug 337481).

Comment 6

[Darin Fisher]

-> me

Comment 7

[Darin Fisher]

Attachment: v1 patch

This seems to do the trick for me.  I think this will break the camino build, which could probably use similar changes.  I'll work on those once we know we are happy with this... or contributions welcome ;-)

Comment 8

[Darin Fisher]

The xpcom-shutdown observer is needed to handle the case where nsAppStartup is not used to run the event loop and no one calls nsIAppShell::Exit.  If we don't set mExiting to true, then we'll get stuck during xpcom shutdown.

Comment 9

[Mark Mentovai]

Comment on attachment 222133 [details] [diff] [review]
v1 patch

Funny - I had just independently diagnosed the cause of the hanging profile manager and was about to e-mail you along with a patch with modifications for carbon and cocoa.

Comment 10

[Mark Mentovai]

Attachment: Cocoa patch

Cocoa patch.  These patches together fix bug 337550.

Comment 11

[Mark Mentovai]

Comment on attachment 222133 [details] [diff] [review]
v1 patch

+  PRBool                      mRunningEventLoop;

PRPackedBool?

Comment 12

[Darin Fisher]

> +  PRBool                      mRunningEventLoop;
> 
> PRPackedBool?

I generally only use PRPackedBool when there are multiple booleans.  Either way, the structure size is going to be the same, and the CPU fetches in word sized chunks, so...

Comment 13

[Darin Fisher]

Comment on attachment 222137 [details] [diff] [review]
Cocoa patch

[NSApp updateWindows] <- how expensive is that?  we assume okay since it would be generated by [NSApp run] anyways?

Comment 14

[Mark Mentovai]

(In reply to comment #13)
> (From update of attachment 222137 [details] [diff] [review] [edit])
> [NSApp updateWindows] <- how expensive is that?  we assume okay since it would
> be generated by [NSApp run] anyways?

Exactly - correctness first.  The default implementation is not terribly expensive.

Comment 15

[Darin Fisher]

fixed-on-trunk

Comment 16

[Darin Fisher]

With this change, I'm observing a 12.5% increase in Txul on my Linux box.  Tp doesn't seem to change.  Ts is regressed by about 5.5%.  I think I need to find a better solution.

Comment 17

[Darin Fisher]

The patches have been backed out of the trunk.

Comment 18

[Darin Fisher]

As expected, reverting the extra OnDispatchedEvent(nsnull) in NativeEventCallback() recovers the lost performance.

Comment 19

[Darin Fisher]

I went ahead and re-landed the patch with the offending bit in NativeEventCallback() commented out.  I'll file a follow-up bug on solving the problem that was trying to solve.  In the meantime, the rest of the patch solves this bug and hopefully resolves the other related Mac bugs.

Marking FIXED again.

Comment 20

[philippe (part-time)]

with the latest re-release of your patch, Camino (own build, optimized gcc 4, 10.4sdk) quits when using command-tab to switch between applications. Console log reports this:

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

Comment 21

[Mark Mentovai]

Cocoa's getting mad that we're dispatching native events after [NSApp terminate:].  I've got a patch, but new problems belong on new bugs.

Comment 22

[philippe (part-time)]

(In reply to comment #21)
> Cocoa's getting mad that we're dispatching native events after [NSApp
> terminate:].  I've got a patch, but new problems belong on new bugs.
> 

Sure:  bug 338249

Comment 23

[Nickolay_Ponomarev]

comment #19:
> I went ahead and re-landed the patch with the offending bit in
> NativeEventCallback() commented out.  I'll file a follow-up bug on solving the
> problem that was trying to solve.

For the reference, the bug filed is bug 338225.