[webkit-reviews] review granted: [Bug 215460] [Cocoa] Avoid changing XPC target queue inside XPC event handler : [Attachment 406618] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Aug 14 14:52:02 PDT 2020


Darin Adler <darin at apple.com> has granted Per Arne Vollan <pvollan at apple.com>'s
request for review:
Bug 215460: [Cocoa] Avoid changing XPC target queue inside XPC event handler
https://bugs.webkit.org/show_bug.cgi?id=215460

Attachment 406618: Patch

https://bugs.webkit.org/attachment.cgi?id=406618&action=review




--- Comment #19 from Darin Adler <darin at apple.com> ---
Comment on attachment 406618
  --> https://bugs.webkit.org/attachment.cgi?id=406618
Patch

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

>
Source/WebKit/Shared/EntryPointUtilities/Cocoa/XPCService/XPCServiceMain.mm:91
> +		   dispatch_sync(dispatch_get_main_queue(),
[initializerFunctionPtr = initializerFunctionPtr, peer = OSObjectPtr(peer),
event = OSObjectPtr(event), priorityBoostMessage =
OSObjectPtr(priorityBoostMessage)] {

The capturing isn’t right here. It’s overzealous.

"initializerFunctionPtr = initializerFunctionPtr" is not different/better than
"initializerFunctionPtr". Given this is dispatch_sync I don’t think you need to
use OSObjectPtr; this thread is holding on to retain counts and is blocked
until dispatch_sync returns.

You could use simpler capturing.

> Source/WebKit/WebProcess/cocoa/HandleXPCEndpointMessages.mm:46
> +#if HAVE(LSDATABASECONTEXT)

Seems like this #if should be around more of the function body. Why wrap it so
tight?


More information about the webkit-reviews mailing list