com.apple.WebKit.Networking.Development crashed in com.apple.WebKit: WebKit::NetworkRTCProvider::resolvedName

[youenn fablet]

com.apple.WebKit.Networking.Development crashed in com.apple.WebKit: WebKit::NetworkRTCProvider::resolvedName.
See rdar 31581795

[youenn fablet]

Created attachment 307235 [details]
Patch

[Sam Weinig]

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.

[Sam Weinig]

If there is a crash, can you provide a test case? If not, can you explain why in the changelog?

[youenn fablet]

(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.

[youenn fablet]

Created attachment 307283 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 307283 [details]
Patch

Clearing flags on attachment: 307283

Committed r215443: <http://trac.webkit.org/changeset/215443>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.