Bug 12718 - REGRESSION(r18184): Segmentation fault when loading abc.go.com
Summary: REGRESSION(r18184): Segmentation fault when loading abc.go.com
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Page Loading (show other bugs)
Version: 420+
Hardware: Mac OS X 10.4
: P1 Critical
Assignee: Nobody
URL: http://abc.go.com
Keywords: InRadar, NeedsReduction, Regression
: 12730 (view as bug list)
Depends on:
Blocks:
 
Reported: 2007-02-09 14:55 PST by Grace Kloba
Modified: 2007-02-13 13:58 PST (History)
3 users (show)

See Also:


Attachments
Patch v1 (no changelog; no test) (609 bytes, patch)
2007-02-11 12:52 PST, David Kilzer (:ddkilzer)
mjs: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Grace Kloba 2007-02-09 14:55:11 PST
Load abc.com with ToT (rev 19542) webkit. Wait until it tries to finish loading all the resources. I got segmentation fault and Safari closed.
Comment 1 David Kilzer (:ddkilzer) 2007-02-09 15:28:49 PST
Confirmed with local debug build of WebKit r19537 with Safari 2.0.4 (419.3) on Mac OS X 10.4.8 (8N1037).

The stack trace does not show WebKit in it, but loading this page does not fail in shipping Safari.  I suspect an over-released ObjC object.

Exception:  EXC_BAD_ACCESS (0x0001)
Codes:      KERN_INVALID_ADDRESS (0x0001) at 0xf3000e17

Thread 0 Crashed:
0   libobjc.A.dylib                	0x90a564c7 objc_msgSend + 23
1   com.apple.Foundation           	0x9265cea5 -[NSURLConnection(NSURLConnectionInternal) _sendCallbacks] + 748
2   com.apple.Foundation           	0x9265cb41 _sendCallbacks + 201
3   com.apple.CoreFoundation       	0x9082afd2 CFRunLoopRunSpecific + 1213
4   com.apple.CoreFoundation       	0x9082ab0e CFRunLoopRunInMode + 61
5   com.apple.HIToolbox            	0x92ddabef RunCurrentEventLoopInMode + 285
6   com.apple.HIToolbox            	0x92dda2fd ReceiveNextEventCommon + 385
7   com.apple.HIToolbox            	0x92dda154 BlockUntilNextEventMatchingListInMode + 81
8   com.apple.AppKit               	0x9327f465 _DPSNextEvent + 572
9   com.apple.AppKit               	0x9327f056 -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] + 137
10  com.apple.Safari               	0x00006cea 0x1000 + 23786
11  com.apple.AppKit               	0x93278ddb -[NSApplication run] + 512
12  com.apple.AppKit               	0x9326cd2f NSApplicationMain + 573
13  com.apple.Safari               	0x0005f54a 0x1000 + 386378
14  com.apple.Safari               	0x0005f471 0x1000 + 386161

Comment 2 mitz 2007-02-09 16:15:55 PST
Here's what I get with zombies enabled:
2007-02-10 02:05:41.791 Safari[14280] *** Selector 'connectionDidFinishLoading:' sent to dealloced instance 0x7df9d00 of class WebCoreResourceHandleAsDelegate.
Break at '-[_NSZombie methodSignatureForSelector:]' to debug.

Comment 3 Maciej Stachowiak 2007-02-10 19:14:59 PST
<rdar://problem/4990049>
Comment 4 David Kilzer (:ddkilzer) 2007-02-11 10:10:52 PST
Doing a binary search of WebKit nightly builds narrows this down to:

r18187	crashes
r18159	does not crash

Comment 5 David Kilzer (:ddkilzer) 2007-02-11 11:26:08 PST
*** Bug 12730 has been marked as a duplicate of this bug. ***
Comment 6 David Kilzer (:ddkilzer) 2007-02-11 11:28:04 PST
(In reply to comment #4)
> Doing a binary search of WebKit nightly builds narrows this down to:
> r18187  crashes
> r18159  does not crash

Creating local builds of r1818x, I narrowed this down to the changes in r18184.  I don't know the loader code well enough at this time to go much further.
Comment 7 David Kilzer (:ddkilzer) 2007-02-11 11:29:37 PST
(In reply to comment #6)
> Creating local builds of r1818x, I narrowed this down to the changes in r18184.

http://trac.webkit.org/projects/webkit/changeset/18184
Comment 8 David Kilzer (:ddkilzer) 2007-02-11 12:52:44 PST
Created attachment 13123 [details]
Patch v1 (no changelog; no test)

This patch fixes the crash, but I haven't figured out what the lifetime of the WebCoreResourceHandleAsDelegate *delegate object is supposed to be (who retains it and who releases it), nor have I worked on creating a test case.
Comment 9 Maciej Stachowiak 2007-02-11 18:12:49 PST
Comment on attachment 13123 [details]
Patch v1 (no changelog; no test)

It's good to know this solves the crash; on the other hand, using autorelease to resolve lifetime issues is somewhat poor form, as it may only be masking the immediate symptom. It's getting put into d->m_delegate, which is a smart pointer, so it should stay alive as long as the ResourceHandle is alive. So that must mean the ResourceHandle actually is getting destroyed too early. How is it that it can get destroyed before connectionDidFinishLoading:?
Comment 10 Brady Eidson 2007-02-12 00:07:29 PST
"How is it that it can get destroyed before connectionDidFinishLoading:?"

Only way I can think of is didFailWithError: - that, or being neglectfully overreleased in some other manner
Comment 11 David Kilzer (:ddkilzer) 2007-02-12 03:39:17 PST
By overriding [WebCoreResourceHandleAsDelegate release], I'm seeing it be released in four places:


#0	0x014cfb04 in -[WebCoreResourceHandleAsDelegate release] at ResourceHandleMac.mm:432
#1	0x929938cc in -[NSURLConnection(NSURLConnectionInternal) _releaseDelegate]
#2	0x92991ab8 in -[NSURLConnection(NSURLConnectionInternal) _sendCallbacks]


#0	0x014cfb04 in -[WebCoreResourceHandleAsDelegate release] at ResourceHandleMac.mm:432
#1	0x92991ac8 in -[NSURLConnection(NSURLConnectionInternal) _sendCallbacks]


#0	0x014cfb04 in -[WebCoreResourceHandleAsDelegate release] at ResourceHandleMac.mm:432
#1	0x014cff80 in WebCore::ResourceHandle::delegate at ResourceHandleMac.mm:138
#2	0x014d02ac in WebCore::ResourceHandle::start at ResourceHandleMac.mm:103


#0	0x014cfb04 in -[WebCoreResourceHandleAsDelegate release] at ResourceHandleMac.mm:432
#1	0x0185c278 in WebCore::RetainPtr<WebCoreResourceHandleAsDelegate>::operator= at RetainPtr.h:131
#2	0x014cfe8c in WebCore::ResourceHandle::releaseDelegate at ResourceHandleMac.mm:148
#3	0x014d0a24 in WebCore::ResourceHandle::~ResourceHandle at ResourceHandleMac.mm:75
#4	0x0185b9dc in WebCore::Shared<WebCore::ResourceHandle>::deref at Shared.h:52
#5	0x01865b34 in WTF::RefPtr<WebCore::ResourceHandle>::operator= at RefPtr.h:106
#6	0x014fc2a4 in WebCore::SubresourceLoader::didFinishLoading at SubresourceLoader.cpp:191
#7	0x014fa2dc in WebCore::ResourceLoader::didFinishLoading at ResourceLoader.cpp:323
#8	0x014cf7c0 in -[WebCoreResourceHandleAsDelegate connectionDidFinishLoading:] at ResourceHandleMac.mm:367

Comment 12 David Kilzer (:ddkilzer) 2007-02-12 03:51:28 PST
By overriding [WebCoreResourceHandleAsDelegate retain], I'm seeing it be
released in 3 places:


#0	0x014cfbac in -[WebCoreResourceHandleAsDelegate retain] at ResourceHandleMac.mm:438
#1	0x0185c244 in WebCore::RetainPtr<WebCoreResourceHandleAsDelegate>::operator= at RetainPtr.h:127
#2	0x014cff64 in WebCore::ResourceHandle::delegate at ResourceHandleMac.mm:137
#3	0x014d0220 in WebCore::ResourceHandle::start at ResourceHandleMac.mm:98


#0	0x014cfbac in -[WebCoreResourceHandleAsDelegate retain] at ResourceHandleMac.mm:438
#1	0x92987da8 in -[NSURLConnection initWithRequest:delegate:priority:]
#2	0x014d0328 in WebCore::ResourceHandle::start at ResourceHandleMac.mm:105


#0	0x014cfbac in -[WebCoreResourceHandleAsDelegate retain] at ResourceHandleMac.mm:438
#1	0x929919cc in -[NSURLConnection(NSURLConnectionInternal) _sendCallbacks]
Comment 13 David Kilzer (:ddkilzer) 2007-02-12 03:52:23 PST
(In reply to comment #12)
> By overriding [WebCoreResourceHandleAsDelegate retain], I'm seeing it be
> released in 3 places:
  ^^^^^^^^

...be retained in 3 places (copy-paste error).
Comment 14 David Kilzer (:ddkilzer) 2007-02-12 03:56:57 PST
This bug only appears to happen with plug-ins, so my current working theory is that a plug-in registers a callback, then gets destroyed, and the process of destroying the plug-in releases the WebCoreResourceHandleAsDelegate one too many times, causing _sendCallBacks to fail when sending a message to a zombie.

The trick is figuring out the over-release path.  (I actually stepped through the whole www.abc.com page load once with a breakpoint on [WebCoreResourceHandleAsDelegate release] until I got the error, but I didn't know exactly when it was coming, so I wasn't paying attention to the call stacks just before the error occurred.  :(
Comment 15 Anders Carlsson 2007-02-13 13:58:08 PST
Committed revision 19612.