Bug 113616

Summary: Crash when "willSendRequest" causes the ResourceLoader to be cancelled.
Product: WebKit Reporter: Brady Eidson <beidson>
Component: WebKit2Assignee: Brady Eidson <beidson>
Status: RESOLVED FIXED    
Severity: Normal CC: ap
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch v1 ap: review+

Brady Eidson
Reported 2013-03-29 15:47:34 PDT
Crash when "willSendRequest" causes the ResourceLoader to be cancelled. This can happen a lot of ways. One example is with the url http://www.rackspace.com/blog/mathematics-cannot-be-patented-case-dismissed/ where an XHR fails a redirect check and therefore the loader is cancelled. When the WebCore loader is cancelled, the WebResourceLoader is destroyed but then it tries to access freed objects. This was only introduced with the asynchronous ResourceHandleClient model so it's very new. By adding a RefPtr<> protector we avoid the crash completely. By adding a "detachFromCoreLoader" to call when the WebCore loader is aborting we can avoid unnecessary messaging back to the NetworkProcess.
Attachments
Patch v1 (5.24 KB, patch)
2013-03-29 15:53 PDT, Brady Eidson
ap: review+
Brady Eidson
Comment 1 2013-03-29 15:53:22 PDT
Created attachment 195811 [details] Patch v1
Alexey Proskuryakov
Comment 2 2013-03-29 15:57:23 PDT
Comment on attachment 195811 [details] Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=195811&action=review > Source/WebKit2/WebProcess/Network/WebResourceLoadScheduler.cpp:161 > + ASSERT(loader); > + if (!loader) > + return; How strongly do you feel about this? We usually do not not add such checks without knowing that this actually happens, and waiting for reproducible cases.
Brady Eidson
Comment 3 2013-03-29 15:59:01 PDT
(In reply to comment #2) > (From update of attachment 195811 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=195811&action=review > > > Source/WebKit2/WebProcess/Network/WebResourceLoadScheduler.cpp:161 > > + ASSERT(loader); > > + if (!loader) > > + return; > > How strongly do you feel about this? We usually do not not add such checks without knowing that this actually happens, and waiting for reproducible cases. I don't feel strongly about it, I'll remove it. (leaving the ASSERT)
Brady Eidson
Comment 4 2013-03-29 16:00:58 PDT
Note You need to log in before you can comment on or make changes to this bug.