Bug 12718

Summary: REGRESSION(r18184): Segmentation fault when loading abc.go.com
Product: WebKit Reporter: Grace Kloba <klobag>
Component: Page LoadingAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Critical CC: andersca, ddkilzer, grhowes
Priority: P1 Keywords: InRadar, NeedsReduction, Regression
Version: 420+   
Hardware: Mac   
OS: OS X 10.4   
URL: http://abc.go.com
Attachments:
Description Flags
Patch v1 (no changelog; no test) mjs: review-

Grace Kloba
Reported 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.
Attachments
Patch v1 (no changelog; no test) (609 bytes, patch)
2007-02-11 12:52 PST, David Kilzer (:ddkilzer)
mjs: review-
David Kilzer (:ddkilzer)
Comment 1 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
mitz
Comment 2 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.
Maciej Stachowiak
Comment 3 2007-02-10 19:14:59 PST
David Kilzer (:ddkilzer)
Comment 4 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
David Kilzer (:ddkilzer)
Comment 5 2007-02-11 11:26:08 PST
*** Bug 12730 has been marked as a duplicate of this bug. ***
David Kilzer (:ddkilzer)
Comment 6 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.
David Kilzer (:ddkilzer)
Comment 7 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
David Kilzer (:ddkilzer)
Comment 8 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.
Maciej Stachowiak
Comment 9 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:?
Brady Eidson
Comment 10 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
David Kilzer (:ddkilzer)
Comment 11 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
David Kilzer (:ddkilzer)
Comment 12 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]
David Kilzer (:ddkilzer)
Comment 13 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).
David Kilzer (:ddkilzer)
Comment 14 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. :(
Anders Carlsson
Comment 15 2007-02-13 13:58:08 PST
Committed revision 19612.
Note You need to log in before you can comment on or make changes to this bug.