Bug 170889 - com.apple.WebKit.Networking.Development crashed in com.apple.WebKit: WebKit::NetworkRTCProvider::resolvedName
Summary: com.apple.WebKit.Networking.Development crashed in com.apple.WebKit: WebKit::...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media Elements (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: youenn fablet
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-04-16 12:33 PDT by youenn fablet
Modified: 2017-04-17 17:20 PDT (History)
3 users (show)

See Also:


Attachments
Patch (2.30 KB, patch)
2017-04-16 12:40 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (2.96 KB, patch)
2017-04-17 11:17 PDT, youenn fablet
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description youenn fablet 2017-04-16 12:33:37 PDT
com.apple.WebKit.Networking.Development crashed in com.apple.WebKit: WebKit::NetworkRTCProvider::resolvedName.
See rdar 31581795
Comment 1 youenn fablet 2017-04-16 12:40:33 PDT
Created attachment 307235 [details]
Patch
Comment 2 Sam Weinig 2017-04-16 13:06:07 PDT
Comment on attachment 307235 [details]
Patch

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

> Source/WebKit2/NetworkProcess/webrtc/NetworkRTCProvider.cpp:166
> +    CFRelease(host);

host should be a RetainPtr<CFHostRef>.  It seems odd that this file is using CFHost, and not an abstraction, since it is not guarded by any COCOA or CORE_FOUNDATION guards.
Comment 3 Sam Weinig 2017-04-16 13:06:50 PDT
If there is a crash, can you provide a test case? If not, can you explain why in the changelog?
Comment 4 youenn fablet 2017-04-16 14:21:25 PDT
(In reply to Sam Weinig from comment #2)
> Comment on attachment 307235 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=307235&action=review
> 
> > Source/WebKit2/NetworkProcess/webrtc/NetworkRTCProvider.cpp:166
> > +    CFRelease(host);
> 
> host should be a RetainPtr<CFHostRef>.  It seems odd that this file is using
> CFHost, and not an abstraction, since it is not guarded by any COCOA or
> CORE_FOUNDATION guards.

I'll update the patch to use RetainPtr.

NetworkRTCProvider is Cocoa specific at the moment.
We will see how to make NetworkRTCProvider abstract when/if other ports will start supporting libwebrtc backend.

Other ports may actually want to directly implement the future model we intent to follow for COCOA: create sockets on the UIProcess, pass them to the WebProcess and do networking in the WebProcess.

(In reply to Sam Weinig from comment #3)
> If there is a crash, can you provide a test case? If not, can you explain
> why in the changelog?

It is difficult to provide a test case.
The crash happens in case the Resolver is destroyed after being stopped.
CFHostCancelInfoResolution is called but sometimes NetworkRTCProvider::resolvedName is called after the resolver being cancelled.
These additional clean-up lines will hopefully make sure things work well.
Comment 5 youenn fablet 2017-04-17 11:17:46 PDT
Created attachment 307283 [details]
Patch
Comment 6 WebKit Commit Bot 2017-04-17 17:14:25 PDT
Comment on attachment 307283 [details]
Patch

Clearing flags on attachment: 307283

Committed r215443: <http://trac.webkit.org/changeset/215443>
Comment 7 WebKit Commit Bot 2017-04-17 17:14:26 PDT
All reviewed patches have been landed.  Closing bug.