Bug 181747 - WebCoreResourceHandleAsOperationQueueDelegate/ResourceHandleCFURLConnectionDelegateWithOperationQueue may be deleted in main thread callback
Summary: WebCoreResourceHandleAsOperationQueueDelegate/ResourceHandleCFURLConnectionDe...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: WebKit Local Build
Hardware: Mac macOS 10.13
: P2 Normal
Assignee: Daniel Bates
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-01-17 10:26 PST by Daniel Bates
Modified: 2018-01-17 10:57 PST (History)
2 users (show)

See Also:

Attachments
Patch (4.89 KB, patch)
2018-01-17 10:35 PST, Daniel Bates 		achristensen: review+
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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>