Changeset 275540 in webkit

Timestamp:

Apr 6, 2021 12:42:13 PM (3 years ago)

Author:

commit-queue@webkit.org

Message:

Hold strong reference to xpc_connection_t in XPCServiceEventHandler
​https://bugs.webkit.org/show_bug.cgi?id=224242
<rdar://74308418>

Patch by Alex Christensen <​achristensen@webkit.org> on 2021-04-06
Reviewed by David Kilzer.

There is a reasonable chance that by the time we get to the block of xpc_connection_set_event_handler
and back through WorkQueue::main().dispatchSync the xpc_connection_t has been freed.

Also, the adoption in each entry points was incorrect, causing a late overrelease when tearing down the process.
It was incorrect because we did not just create the connection or get it from any NS_RETURNS_RETAINED-like function.
It was causing rare crashes once everything else had released their hold on the connection.

• GPUProcess/EntryPoint/Cocoa/XPCService/GPUServiceEntryPoint.mm:

(GPU_SERVICE_INITIALIZER):

• NetworkProcess/EntryPoint/Cocoa/XPCService/NetworkServiceEntryPoint.mm:

(NETWORK_SERVICE_INITIALIZER):

• PluginProcess/EntryPoint/Cocoa/XPCService/PluginServiceEntryPoint.mm:

(PLUGIN_SERVICE_INITIALIZER):

• Shared/EntryPointUtilities/Cocoa/XPCService/XPCServiceMain.mm:

(WebKit::XPCServiceEventHandler):

• WebAuthnProcess/EntryPoint/Cocoa/XPCService/WebAuthnServiceEntryPoint.mm:

(WEBAUTHN_SERVICE_INITIALIZER):

• WebProcess/EntryPoint/Cocoa/XPCService/WebContentServiceEntryPoint.mm:

(WEBCONTENT_SERVICE_INITIALIZER):

Location:

trunk/Source/WebKit

Files:

• ChangeLog (modified) (1 diff)
• GPUProcess/EntryPoint/Cocoa/XPCService/GPUServiceEntryPoint.mm (modified) (1 diff)
• NetworkProcess/EntryPoint/Cocoa/XPCService/NetworkServiceEntryPoint.mm (modified) (1 diff)
• PluginProcess/EntryPoint/Cocoa/XPCService/PluginServiceEntryPoint.mm (modified) (1 diff)
• Shared/EntryPointUtilities/Cocoa/XPCService/XPCServiceMain.mm (modified) (2 diffs)
• WebAuthnProcess/EntryPoint/Cocoa/XPCService/WebAuthnServiceEntryPoint.mm (modified) (1 diff)
• WebProcess/EntryPoint/Cocoa/XPCService/WebContentServiceEntryPoint.mm (modified) (1 diff)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2021-04-06  Alex Christensen  <achristensen@webkit.org>
+
+        Hold strong reference to xpc_connection_t in XPCServiceEventHandler
+        https://bugs.webkit.org/show_bug.cgi?id=224242
+        <rdar://74308418>
+
+        Reviewed by David Kilzer.
+
+        There is a reasonable chance that by the time we get to the block of xpc_connection_set_event_handler
+        and back through WorkQueue::main().dispatchSync the xpc_connection_t has been freed.
+
+        Also, the adoption in each entry points was incorrect, causing a late overrelease when tearing down the process.
+        It was incorrect because we did not just create the connection or get it from any NS_RETURNS_RETAINED-like function.
+        It was causing rare crashes once everything else had released their hold on the connection.
+
+        * GPUProcess/EntryPoint/Cocoa/XPCService/GPUServiceEntryPoint.mm:
+        (GPU_SERVICE_INITIALIZER):
+        * NetworkProcess/EntryPoint/Cocoa/XPCService/NetworkServiceEntryPoint.mm:
+        (NETWORK_SERVICE_INITIALIZER):
+        * PluginProcess/EntryPoint/Cocoa/XPCService/PluginServiceEntryPoint.mm:
+        (PLUGIN_SERVICE_INITIALIZER):
+        * Shared/EntryPointUtilities/Cocoa/XPCService/XPCServiceMain.mm:
+        (WebKit::XPCServiceEventHandler):
+        * WebAuthnProcess/EntryPoint/Cocoa/XPCService/WebAuthnServiceEntryPoint.mm:
+        (WEBAUTHN_SERVICE_INITIALIZER):
+        * WebProcess/EntryPoint/Cocoa/XPCService/WebContentServiceEntryPoint.mm:
+        (WEBCONTENT_SERVICE_INITIALIZER):
+
 2021-04-06  Brent Fulgham  <bfulgham@apple.com>
 

trunk/Source/WebKit/GPUProcess/EntryPoint/Cocoa/XPCService/GPUServiceEntryPoint.mm

@@ -60 +60 @@
 
 #if ENABLE(GPU_PROCESS)
-    WebKit::XPCServiceInitializer<WebKit::GPUProcess, WebKit::GPUServiceInitializerDelegate>(adoptOSObject(connection), initializerMessage, priorityBoostMessage);
+    WebKit::XPCServiceInitializer<WebKit::GPUProcess, WebKit::GPUServiceInitializerDelegate>(connection, initializerMessage, priorityBoostMessage);
 #endif // ENABLE(GPU_PROCESS)
 }

trunk/Source/WebKit/NetworkProcess/EntryPoint/Cocoa/XPCService/NetworkServiceEntryPoint.mm

@@ -56 +56 @@
 {
     WTF::initializeMainThread();
-    XPCServiceInitializer<NetworkProcess, NetworkServiceInitializerDelegate>(adoptOSObject(connection), initializerMessage, priorityBoostMessage);
+    XPCServiceInitializer<NetworkProcess, NetworkServiceInitializerDelegate>(connection, initializerMessage, priorityBoostMessage);
 }

trunk/Source/WebKit/PluginProcess/EntryPoint/Cocoa/XPCService/PluginServiceEntryPoint.mm

@@ -82 +82 @@
     // spawned by the PluginProcess don't try to insert the shim and crash.
     EnvironmentUtilities::removeValuesEndingWith("DYLD_INSERT_LIBRARIES", "/PluginProcessShim.dylib");
-    XPCServiceInitializer<PluginProcess, PluginServiceInitializerDelegate>(adoptOSObject(connection), initializerMessage, priorityBoostMessage);
+    XPCServiceInitializer<PluginProcess, PluginServiceInitializerDelegate>(connection, initializerMessage, priorityBoostMessage);
 #endif // ENABLE(NETSCAPE_PLUGIN_API)
 }

trunk/Source/WebKit/Shared/EntryPointUtilities/Cocoa/XPCService/XPCServiceMain.mm

@@ -65 +65 @@
     static NeverDestroyed<OSObjectPtr<xpc_object_t>> priorityBoostMessage;
 
+    OSObjectPtr<xpc_connection_t> retainedPeerConnection(peer);
+
     xpc_connection_set_target_queue(peer, dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0));
     xpc_connection_set_event_handler(peer, ^(xpc_object_t event) {
@@ -118 +120 @@
                     dup2(fd, STDERR_FILENO);
 
-                WorkQueue::main().dispatchSync([&] {
-                    initializerFunctionPtr(peer, event, priorityBoostMessage.get().get());
+                WorkQueue::main().dispatchSync([initializerFunctionPtr, event = OSObjectPtr<xpc_object_t>(event), retainedPeerConnection] {
+                    initializerFunctionPtr(retainedPeerConnection.get(), event.get(), priorityBoostMessage.get().get());
 
                     setAppleLanguagesPreference();

trunk/Source/WebKit/WebAuthnProcess/EntryPoint/Cocoa/XPCService/WebAuthnServiceEntryPoint.mm

@@ -60 +60 @@
 
 #if ENABLE(WEB_AUTHN)
-    WebKit::XPCServiceInitializer<WebKit::WebAuthnProcess, WebKit::WebAuthnServiceInitializerDelegate>(adoptOSObject(connection), initializerMessage, priorityBoostMessage);
+    WebKit::XPCServiceInitializer<WebKit::WebAuthnProcess, WebKit::WebAuthnServiceInitializerDelegate>(connection, initializerMessage, priorityBoostMessage);
 #endif // ENABLE(WEB_AUTHN)
 }

trunk/Source/WebKit/WebProcess/EntryPoint/Cocoa/XPCService/WebContentServiceEntryPoint.mm

@@ -51 +51 @@
 #endif // PLATFORM(IOS_FAMILY)
 
-    WebKit::XPCServiceInitializer<WebKit::WebProcess, WebKit::XPCServiceInitializerDelegate>(adoptOSObject(connection), initializerMessage, priorityBoostMessage);
+    WebKit::XPCServiceInitializer<WebKit::WebProcess, WebKit::XPCServiceInitializerDelegate>(connection, initializerMessage, priorityBoostMessage);
 }