420223 - Add |setDelegate:nil| call in the |nsMenuBarX| destructor.

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect, P2)

Description

[Nick Kreeger]

Attachment: Patch V1

We are seeing a deallocated |MessageDelegate| object getting sent the |menuNeedsUpdate:| message.

 We see this when Songbird closes the player window to re-apply a new feather (or skin). While the window is closing and the new resources are getting loaded - if a user clicks on the menubar, we get a SIGTRAP crash. Running the build with |NSZombieEnabled| showed this:

2008-02-28 18:13:49.315 songbird[56131:10b] *** -[MenuDelegate menuNeedsUpdate:]: message sent to deallocated instance 0x438722b0

The sample stack trace looks like this:
#0	0x96cc96ec in objc_msgSend
#1	0x92067484 in -[NSMenu _populate:]
#2	0x920673ae in AppKitMenuEventHandler
#3	0x931c7fc3 in DispatchEventToHandlers
#4	0x931c73fd in SendEventToEventTargetInternal
#5	0x931c7262 in SendEventToEventTargetWithOptions
#6	0x9320c675 in SendMenuPopulate
#7	0x93221563 in SendMenuOpening
#8	0x93221177 in DrawTheMenu
#9	0x93220f53 in MenuChanged
#10	0x9321fdaa in TrackMenuCommon
#11	0x9321b9b5 in MenuSelectCore
#12	0x9321b509 in _HandleMenuSelection2
#13	0x9321b37d in _HandleMenuSelection
#14	0x91fa3f77 in _NSHandleCarbonMenuEvent
#15	0x91f0ac72 in _DPSNextEvent
#16	0x91f0a08e in -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:]

This can easily be fixed by setting the |mMacMenu| delegate to nil:
[mMacMenu setDelegate:nil];

See the attached patch.

Comment 1

[Håkan Waara]

Nice find kreeger. I used to see this crash in Firefox as well when "Cocoafox" was still experimental.

Comment 2

[Josh Aas]

Comment on attachment 306422 [details] [diff] [review]
Patch V1

This is very safe protection against a crash.

Comment 3

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 306422 [details] [diff] [review]
Patch V1

a1.9b4=beltzner

Comment 4

[Nick Kreeger]

Checked into trunk:

/cvsroot/mozilla/widget/src/cocoa/nsMenuX.mm,v  <--  nsMenuX.mm
new revision: 1.63; previous revision: 1.62

Thanks to all for the quick turn around on this!