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

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>