704866 - Firefox Crash [@ firefox@0x1 ] [@ -[NativeMenuItemTarget menuItemHit:] ]

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect)

Description

[Marcia Knous [:marcia]]

Seen while looking at crash stats. Both sigs seem to be related. First signature appears in 8.0 and 8.01, Mac only - https://crash-stats.mozilla.com/report/list?signature=firefox@0x1. Most crashes appear to be startup crashes.

Frame 	Module 	Signature [Expand] 	Source
0 	firefox 	firefox@0x1 	
1 	XUL 	-[NativeMenuItemTarget menuItemHit:] 	widget/src/cocoa/nsMenuBarX.mm:857
2 	AppKit 	-[NSApplication sendAction:to:from:] 	
3 	AppKit 	-[NSMenuItem _corePerformAction] 	
4 	AppKit 	-[NSCarbonMenuImpl performActionWithHighlightingForItemAtIndex:] 	
5 	AppKit 	-[NSMenu performKeyEquivalent:] 	
6 	XUL 	-[ChildView keyDown:] 	widget/src/cocoa/nsChildView.mm:3962
7 	AppKit 	-[NSWindow sendEvent:] 	
8 	XUL 	-[ToolbarWindow sendEvent:] 	widget/src/cocoa/nsCocoaWindow.mm:2396
9 	AppKit 	-[NSApplication sendEvent:] 	
10 	XUL 	-[GeckoNSApplication sendEvent:] 	widget/src/cocoa/nsAppShell.mm:192
11 	AppKit 	-[NSApplication run] 	
12 	XUL 	nsAppShell::Run 	widget/src/cocoa/nsAppShell.mm:771
13 	XUL 	nsAppStartup::Run 	toolkit/components/startup/nsAppStartup.cpp:224
14 	XUL 	XRE_main 	toolkit/xre/nsAppRunner.cpp:3544
15 	firefox 	main 	browser/app/nsBrowserApp.cpp:198
16 	firefox 	firefox@0xac3 

The other signature https://crash-stats.mozilla.com/report/list?signature=-[NativeMenuItemTarget%20menuItemHit:] seems to have a mix of Firefox and Thunderbird crashes but a very similar signature:


https://crash-stats.mozilla.com/report/index/d1e1efb1-bddd-4514-a5f4-071822111118

Frame 	Module 	Signature [Expand] 	Source
0 	XUL 	-[NativeMenuItemTarget menuItemHit:] 	widget/src/cocoa/nsMenuBarX.mm:857
1 	AppKit 	-[NSApplication sendAction:to:from:] 	
2 	AppKit 	-[NSMenuItem _corePerformAction] 	
3 	AppKit 	-[NSCarbonMenuImpl performActionWithHighlightingForItemAtIndex:] 	
4 	AppKit 	-[NSMenu _internalPerformActionForItemAtIndex:] 	
5 	AppKit 	-[NSCarbonMenuImpl _carbonCommandProcessEvent:handlerCallRef:] 	
6 	AppKit 	NSSLMMenuEventHandler 	
7 	HIToolbox 	HIToolbox@0x77f6 	
8 	HIToolbox 	HIToolbox@0x6d45 	
9 	HIToolbox 	HIToolbox@0x24a80 	
10 	HIToolbox 	HIToolbox@0x53c34 	
11 	HIToolbox 	HIToolbox@0x80a09 	
12 	HIToolbox 	HIToolbox@0x809c1 	
13 	HIToolbox 	HIToolbox@0x808d1 	
14 	HIToolbox 	HIToolbox@0x61c26 	
15 	HIToolbox 	HIToolbox@0x6137b 	
16 	AppKit 	_NSHandleCarbonMenuEvent 	
17 	AppKit 	_DPSNextEvent 	
18 	AppKit 	-[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] 	
19 	AppKit 	-[NSApplication run] 	
20 	XUL 	nsAppShell::Run 	widget/src/cocoa/nsAppShell.mm:746
21 	XUL 	nsAppStartup::Run 	toolkit/components/startup/nsAppStartup.cpp:218
22 	XUL 	XRE_main 	toolkit/xre/nsAppRunner.cpp:3754
23 	firefox-bin 	main 	browser/app/nsBrowserApp.cpp:158
24 	firefox-bin 	firefox-bin@0x953

Comment 1

[Steven Michaud [:smichaud] (Retired)]

These crashes go back at least as far as FF 4.

They're low volume -- 96 in the last 4 weeks.

I'd guess they're happening on access to a deleted NativeMenuItemTarget object.  (Note that a number of comments report crashes on quit.)

It'll probably be a while before I can get to this.

Comment 2

[Benoit Girard (:BenWa)]

Should we ducktape this crash away with some aggressive null check in the mean time?

Comment 3

[Steven Michaud [:smichaud] (Retired)]

I don't know, and it'll take me at least an hour or two digging through the code to find out.

But access to bugzilla has been troublesome all day, which blocks the rest of my work.  So I'll spend the next couple of hours looking into this :-)

Comment 4

[Steven Michaud [:smichaud] (Retired)]

Attachment: Hacky fix

In my enforced "idleness", I've been able to figure out the likely
cause of this bug, and how to fix it.

The crashes aren't caused by accessing a deleted NativeMenuItemTarget
object (we appear to never delete those).  Instead it seems that
menuGroupOwner (an nsMenuGroupOwnerX object) might have been deleted.
An nsMenuGroupOwnerX object is always also an nsMenuBarX object, which
(as a comment warns above its class definition in nsMenuBarX.h)	can
become invalid whenever its DOM node is destroyed.

This patch is a hack.  As my patch comment says, fixing this bug
properly might trigger delays as windows/tabs are closed.  My hack
avoids these (potential) delays.  If it doesn't fix	enough of these
crashes, or if it causes problems of its own, we'll need to risk the
delays.  But I think it's worth trying my hack out on the trunk for a
while, to see if it helps.

Comment 5

[Steven Michaud [:smichaud] (Retired)]

Landed on mozilla-inbound:
http://hg.mozilla.org/integration/mozilla-inbound/rev/6552a4bba94a

Comment 6

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/6552a4bba94a

Comment 7

[Steven Michaud [:smichaud] (Retired)]

There haven't been any of these crashes on the trunk since my patch landed.  So it seems to be working.