300995 - Crash when dragging bookmark buttons to bookmarks bar with SDK >= 10.3

Status: RESOLVED FIXED

(Camino Graveyard :: Drag & Drop, defect)

Description

[Mark Mentovai]

When Camino is built with SDK >= 10.3, the following crash is encountered
whenever dragging a bookmark button from the bookmarks bar and then droppping it
in the bookmarks bar.  The crash occurs after the the dragging destination
completes performDragOperation:.

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_PROTECTION_FAILURE at address: 0x00000000
0xfffeff20 in objc_msgSend_rtp ()
(gdb) bt
#0  0xfffeff20 in objc_msgSend_rtp ()
#1  0x9388fd2c in -[NSCoreDragManager _dragUntilMouseUp:accepted:] ()
#2  0x9388f488 in -[NSCoreDragManager
dragImage:fromWindow:at:offset:event:pasteboard:source:slideBack:] ()
#3  0x9388f014 in -[NSWindow(NSDrag)
dragImage:at:offset:event:pasteboard:source:slideBack:] ()
#4  0x9388ef78 in -[NSView(NSDrag)
dragImage:at:offset:event:pasteboard:source:slideBack:] ()
#5  0x00060f28 in -[BookmarkButton mouseDragged:] (self=0xb441570,
_cmd=0x90a188d0, aEvent=0xb449c10) at
/Developer/SDKs/MacOSX10.4.0.sdk/System/Library/Frameworks/Foundation.framework/Headers/NSGeometry.h:52
#6  0x93688438 in -[NSWindow sendEvent:] ()
#7  0x0003e914 in -[BrowserWindow sendEvent:] (self=0xe993a0, _cmd=0x90a188d0,
theEvent=0xb449c10) at
/Users/mark/lizard/trunk/build/camino-debug/camino/src/browser/BrowserWindow.mm:76
#8  0x93630f5c in -[NSApplication sendEvent:] ()
#9  0x936283f0 in -[NSApplication run] ()
#10 0x93718c1c in NSApplicationMain ()
#11 0x00009318 in main (argc=1, argv=0xbffff71c) at
/Users/mark/lizard/trunk/build/camino-debug/camino/src/application/main.m:53

Comment 1

[Simon Fraser [no longer active]]

This looks like one for Apple.

Comment 2

[Mark Mentovai]

Weird.  The release comes from them.

Breakpoint 1, -[BookmarkButton dealloc] (self=0xb2dfe00, _cmd=0x909be2cc) at
/Users/mark/lizard/trunk/build/camino-debug/camino/src/bookmarks/BookmarkButton.mm:93
93      fprintf(stderr,"deallocing button\n");
(gdb) bt
#0  -[BookmarkButton dealloc] (self=0xb2dfe00, _cmd=0x909be2cc) at
/Users/mark/lizard/trunk/build/camino-debug/camino/src/bookmarks/BookmarkButton.mm:93
#1  0x9363cacc in -[NSView release] ()
#2  0x92859708 in NSPopAutoreleasePool ()
#3  0x93889fb0 in NSCoreDragReceiveProc ()
#4  0x917aea0c in DoDropMessage ()
#5  0x917ae97c in SendDropMessage ()
#6  0x917ad078 in DragInApplication ()
#7  0x917ac9dc in DragLoop ()
#8  0x917abae0 in CoreDragStartDragging ()
#9  0x9388fc0c in -[NSCoreDragManager _dragUntilMouseUp:accepted:] ()
#10 0x9388f488 in -[NSCoreDragManager
dragImage:fromWindow:at:offset:event:pasteboard:source:slideBack:] ()
#11 0x9388f014 in -[NSWindow(NSDrag)
dragImage:at:offset:event:pasteboard:source:slideBack:] ()
#12 0x9388ef78 in -[NSView(NSDrag)
dragImage:at:offset:event:pasteboard:source:slideBack:] ()
#13 0x00060f58 in -[BookmarkButton mouseDragged:] (self=0xb2dfe00,
_cmd=0x909be2cc, aEvent=0xb2ac4a0) at
/Developer/SDKs/MacOSX10.4.0.sdk/System/Library/Frameworks/Foundation.framework/Headers/NSGeometry.h:52
#14 0x93688438 in -[NSWindow sendEvent:] ()
#15 0x0003e92c in -[BrowserWindow sendEvent:] (self=0xe9e840, _cmd=0x909be2cc,
theEvent=0xb2ac4a0) at
/Users/mark/lizard/trunk/build/camino-debug/camino/src/browser/BrowserWindow.mm:76
#16 0x93630f5c in -[NSApplication sendEvent:] ()
#17 0x936283f0 in -[NSApplication run] ()
#18 0x93718c1c in NSApplicationMain ()
#19 0x00009330 in main (argc=1, argv=0xbffff71c) at
/Users/mark/lizard/trunk/build/camino-debug/camino/src/application/main.m:55

Comment 3

[Mark Mentovai]

Attachment: Fix

Actually, it looks like our problem, but it's being masked when built against
SDK 10.2.8 because of a difference in cleanup strategy.  A bookmark button drag
may result in the button being removed (or moved, resulting in a removal) which
causes deallocation.  The dealloc can come before the system's done sending
messages to the object.

I'm in the process of testing this with a "standard" 10.2.8 SDK build.

Comment 4

[Simon Fraser [no longer active]]

You could use the sly [[self retain] autorelease] here also.

Comment 5

[Mark Mentovai]

Attachment: With autorelease

Cool.  That was actually the first thing I tried, but I thought I'd get hung.

This is fine with the standard 10.2.8 PPC build, where the refcounts aren't
quite as stinky.  I made sure that the button object gets dealloced regardless
of SDK.

This brings us one step closer to x86 Camino.

Comment 6

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 189551 [details] [diff] [review]
With autorelease

r=pink

Comment 7

[Mark Mentovai]

In.