Bug 181747

Summary: WebCoreResourceHandleAsOperationQueueDelegate/ResourceHandleCFURLConnectionDelegateWithOperationQueue may be deleted in main thread callback
Product: WebKit Reporter: Daniel Bates <dbates>
Component: PlatformAssignee: Daniel Bates <dbates>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Local Build   
Hardware: Mac   
OS: macOS 10.13   
Attachments:
Description Flags
Patch achristensen: review+

Description Daniel Bates 2018-01-17 10:26:31 PST
While investigating the assertion failure in bug #181746 I noticed that WebCoreResourceHandleAsOperationQueueDelegate does not retain itself before waiting on a main thread operation. The main thread operation can do anything, including detaching from WebCoreResourceHandleAsOperationQueueDelegate and deleting it. A PingHandle is one example of a resource handle that will delete itself as soon as possible => detach and delete its resource handle delegate (WebCoreResourceHandleAsOperationQueueDelegate). Specifically, a PingHandle will delete itself when the delegate queries (on the main thread) whether it can respond to an authentication request (a ping never responds to authentication requests) => WebCoreResourceHandleAsOperationQueueDelegate is deleted while it is waiting for the main thread to respond.
Comment 1 Radar WebKit Bug Importer 2018-01-17 10:33:57 PST
<rdar://problem/36588120>
Comment 2 Daniel Bates 2018-01-17 10:35:02 PST
Created attachment 331510 [details]
Patch
Comment 3 Alex Christensen 2018-01-17 10:38:38 PST
Comment on attachment 331510 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=331510&action=review

> Source/WebCore/platform/network/mac/WebCoreResourceHandleAsOperationQueueDelegate.mm:172
> +    auto protectedSelf = retainPtr(self);

I'm not sure if this one's necessary because it doesn't do anything with self after calling the function which, until it is sent to the main thread, has a protector inside it's lambda capture.
Comment 4 Alex Christensen 2018-01-17 10:38:57 PST
We should consider doing the same for the CFURLConnection code on Windows.
Comment 5 Daniel Bates 2018-01-17 10:40:19 PST
Comment on attachment 331510 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=331510&action=review

>> Source/WebCore/platform/network/mac/WebCoreResourceHandleAsOperationQueueDelegate.mm:172
>> +    auto protectedSelf = retainPtr(self);
> 
> I'm not sure if this one's necessary because it doesn't do anything with self after calling the function which, until it is sent to the main thread, has a protector inside it's lambda capture.

Oops! Will remove.
Comment 6 Daniel Bates 2018-01-17 10:48:42 PST
(In reply to Alex Christensen from comment #4)
> We should consider doing the same for the CFURLConnection code on Windows.

Will do before landing.
Comment 7 Daniel Bates 2018-01-17 10:57:21 PST
Committed r227073: <https://trac.webkit.org/changeset/227073>