- Revision
- 265715
- Author
- [email protected]
- Date
- 2020-08-14 16:27:26 -0700 (Fri, 14 Aug 2020)
Log Message
[Cocoa] Avoid changing XPC target queue inside XPC event handler
https://bugs.webkit.org/show_bug.cgi?id=215460
Reviewed by Darin Adler.
In WebProcess::handleXPCEndpointMessages we currently change the XPC target queue for the XPC bootstrap connection while
under the XPC event handler. This sometimes causes simulated crashes on iOS and should be avoided. According to the
documentation in https://developer.apple.com/documentation/xpc/1448786-xpc_connection_set_target_queue?language=objc,
there does not seem to be anything saying this is a programming error, but the simulated crash claims otherwise. This
patch addresses this issue by changing the initial target queue for the XPC bootstrap connection from the main thread
queue to a queue on a secondary thread. The WebKit initializer function needs to be called on the main thread, which
is done by synchronously dispatching the call on the main thread. This patch also stops changing the event handler for
the bootstrap connection, but instead adds the additional XPC handling to the current event handler.
No new tests, since this is covered by existing tests. The handling of the Launch Services XPC endpoint message that
this patch changes, is used to receive the Launch Services database from the Networking process. The Launch Services
database is used in MIME type mapping APIs, and many tests would fail if this database was not received in the
WebContent process.
* Shared/EntryPointUtilities/Cocoa/XPCService/XPCServiceMain.mm:
(WebKit::XPCServiceEventHandler):
* WebKit.xcodeproj/project.pbxproj:
* WebProcess/WebPage/Cocoa/WebPageCocoa.mm:
(WebKit::WebPage::platformDidReceiveLoadParameters):
* WebProcess/WebProcess.cpp:
(WebKit::WebProcess::initializeConnection):
* WebProcess/WebProcess.h:
* WebProcess/cocoa/HandleXPCEndpointMessages.h: Added.
* WebProcess/cocoa/HandleXPCEndpointMessages.mm: Added.
(WebKit::handleXPCEndpointMessages):
* WebProcess/cocoa/WebProcessCocoa.mm:
(WebKit::WebProcess::handleXPCEndpointMessages const): Deleted.
Modified Paths
Added Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (265714 => 265715)
--- trunk/Source/WebKit/ChangeLog 2020-08-14 23:09:54 UTC (rev 265714)
+++ trunk/Source/WebKit/ChangeLog 2020-08-14 23:27:26 UTC (rev 265715)
@@ -1,3 +1,38 @@
+2020-08-14 Per Arne Vollan <[email protected]>
+
+ [Cocoa] Avoid changing XPC target queue inside XPC event handler
+ https://bugs.webkit.org/show_bug.cgi?id=215460
+
+ Reviewed by Darin Adler.
+
+ In WebProcess::handleXPCEndpointMessages we currently change the XPC target queue for the XPC bootstrap connection while
+ under the XPC event handler. This sometimes causes simulated crashes on iOS and should be avoided. According to the
+ documentation in https://developer.apple.com/documentation/xpc/1448786-xpc_connection_set_target_queue?language=objc,
+ there does not seem to be anything saying this is a programming error, but the simulated crash claims otherwise. This
+ patch addresses this issue by changing the initial target queue for the XPC bootstrap connection from the main thread
+ queue to a queue on a secondary thread. The WebKit initializer function needs to be called on the main thread, which
+ is done by synchronously dispatching the call on the main thread. This patch also stops changing the event handler for
+ the bootstrap connection, but instead adds the additional XPC handling to the current event handler.
+
+ No new tests, since this is covered by existing tests. The handling of the Launch Services XPC endpoint message that
+ this patch changes, is used to receive the Launch Services database from the Networking process. The Launch Services
+ database is used in MIME type mapping APIs, and many tests would fail if this database was not received in the
+ WebContent process.
+
+ * Shared/EntryPointUtilities/Cocoa/XPCService/XPCServiceMain.mm:
+ (WebKit::XPCServiceEventHandler):
+ * WebKit.xcodeproj/project.pbxproj:
+ * WebProcess/WebPage/Cocoa/WebPageCocoa.mm:
+ (WebKit::WebPage::platformDidReceiveLoadParameters):
+ * WebProcess/WebProcess.cpp:
+ (WebKit::WebProcess::initializeConnection):
+ * WebProcess/WebProcess.h:
+ * WebProcess/cocoa/HandleXPCEndpointMessages.h: Added.
+ * WebProcess/cocoa/HandleXPCEndpointMessages.mm: Added.
+ (WebKit::handleXPCEndpointMessages):
+ * WebProcess/cocoa/WebProcessCocoa.mm:
+ (WebKit::WebProcess::handleXPCEndpointMessages const): Deleted.
+
2020-08-14 Kate Cheney <[email protected]>
Convert SharedMemory::Handle to SharedMemory::IPCHandle WebPage image/pasteboard functions
Modified: trunk/Source/WebKit/Shared/EntryPointUtilities/Cocoa/XPCService/XPCServiceMain.mm (265714 => 265715)
--- trunk/Source/WebKit/Shared/EntryPointUtilities/Cocoa/XPCService/XPCServiceMain.mm 2020-08-14 23:09:54 UTC (rev 265714)
+++ trunk/Source/WebKit/Shared/EntryPointUtilities/Cocoa/XPCService/XPCServiceMain.mm 2020-08-14 23:27:26 UTC (rev 265715)
@@ -25,6 +25,7 @@
#import "config.h"
+#import "HandleXPCEndpointMessages.h"
#import "WKCrashReporter.h"
#import "XPCServiceEntryPoint.h"
#import <CoreFoundation/CoreFoundation.h>
@@ -41,7 +42,7 @@
{
static xpc_object_t priorityBoostMessage = nullptr;
- xpc_connection_set_target_queue(peer, dispatch_get_main_queue());
+ 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) {
xpc_type_t type = xpc_get_type(event);
if (type == XPC_TYPE_ERROR) {
@@ -87,7 +88,10 @@
if (fd != -1)
dup2(fd, STDERR_FILENO);
- initializerFunctionPtr(peer, event, priorityBoostMessage);
+ dispatch_sync(dispatch_get_main_queue(), ^{
+ initializerFunctionPtr(peer, event, priorityBoostMessage);
+ });
+
if (priorityBoostMessage)
xpc_release(priorityBoostMessage);
}
@@ -97,6 +101,8 @@
assert(!priorityBoostMessage);
priorityBoostMessage = xpc_retain(event);
}
+
+ handleXPCEndpointMessages(event);
}
});
Modified: trunk/Source/WebKit/WebKit.xcodeproj/project.pbxproj (265714 => 265715)
--- trunk/Source/WebKit/WebKit.xcodeproj/project.pbxproj 2020-08-14 23:09:54 UTC (rev 265714)
+++ trunk/Source/WebKit/WebKit.xcodeproj/project.pbxproj 2020-08-14 23:27:26 UTC (rev 265715)
@@ -1692,6 +1692,7 @@
C1663E5B24AEAA2F00C6A3B2 /* LaunchServicesDatabaseXPCConstants.h in Headers */ = {isa = PBXBuildFile; fileRef = C1663E5A24AEA74200C6A3B2 /* LaunchServicesDatabaseXPCConstants.h */; };
C1710CF724AA643200D7C112 /* LaunchServicesDatabaseObserver.mm in Sources */ = {isa = PBXBuildFile; fileRef = C1710CF624AA643200D7C112 /* LaunchServicesDatabaseObserver.mm */; };
C18173612058424700DFDA65 /* DisplayLink.h in Headers */ = {isa = PBXBuildFile; fileRef = C18173602058424700DFDA65 /* DisplayLink.h */; };
+ C1A152D724E5A29A00978C8B /* HandleXPCEndpointMessages.mm in Sources */ = {isa = PBXBuildFile; fileRef = C1A152D624E5A29A00978C8B /* HandleXPCEndpointMessages.mm */; };
C1E123BA20A11573002646F4 /* PDFContextMenu.h in Headers */ = {isa = PBXBuildFile; fileRef = C1E123B920A11572002646F4 /* PDFContextMenu.h */; };
C517388112DF8F4F00EE3F47 /* DragControllerAction.h in Headers */ = {isa = PBXBuildFile; fileRef = C517388012DF8F4F00EE3F47 /* DragControllerAction.h */; };
C54256B518BEC18C00DE4179 /* WKDateTimeInputControl.h in Headers */ = {isa = PBXBuildFile; fileRef = C54256AF18BEC18B00DE4179 /* WKDateTimeInputControl.h */; };
@@ -5036,6 +5037,8 @@
C1817362205844A900DFDA65 /* DisplayLink.cpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path = DisplayLink.cpp; sourceTree = "<group>"; };
C18FB51D242F9F76007E9875 /* WebSleepDisablerClient.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = WebSleepDisablerClient.cpp; sourceTree = "<group>"; };
C18FB51E242F9F77007E9875 /* WebSleepDisablerClient.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = WebSleepDisablerClient.h; sourceTree = "<group>"; };
+ C1A152D524E5A1D200978C8B /* HandleXPCEndpointMessages.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = HandleXPCEndpointMessages.h; sourceTree = "<group>"; };
+ C1A152D624E5A29A00978C8B /* HandleXPCEndpointMessages.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = HandleXPCEndpointMessages.mm; sourceTree = "<group>"; };
C1E123B920A11572002646F4 /* PDFContextMenu.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = PDFContextMenu.h; sourceTree = "<group>"; };
C1ED723724A5690E003F6CD3 /* NetworkProcessProxyCocoa.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = NetworkProcessProxyCocoa.mm; sourceTree = "<group>"; };
C517388012DF8F4F00EE3F47 /* DragControllerAction.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = DragControllerAction.h; sourceTree = "<group>"; };
@@ -8384,6 +8387,8 @@
children = (
CD4570CB2440FB2A00A3DCEB /* AudioSessionRoutingArbitrator.cpp */,
CD4570CA2440FB2A00A3DCEB /* AudioSessionRoutingArbitrator.h */,
+ C1A152D524E5A1D200978C8B /* HandleXPCEndpointMessages.h */,
+ C1A152D624E5A29A00978C8B /* HandleXPCEndpointMessages.mm */,
C14D37FC24ACDF45007FF014 /* LaunchServicesDatabaseManager.h */,
C14D37FD24ACE086007FF014 /* LaunchServicesDatabaseManager.mm */,
446DC64B24A2D8AD0061F390 /* PlaybackSessionContextIdentifier.h */,
@@ -12742,6 +12747,7 @@
CDCDC99E248FE8DA00A69522 /* EndowmentStateTracker.mm in Sources */,
1AA576021496B97900A4EE06 /* EventDispatcherMessageReceiver.cpp in Sources */,
CDA93DB122F8BCF400490A69 /* FullscreenTouchSecheuristicParameters.cpp in Sources */,
+ C1A152D724E5A29A00978C8B /* HandleXPCEndpointMessages.mm in Sources */,
2749F6442146561B008380BF /* InjectedBundleNodeHandle.cpp in Sources */,
2749F6452146561E008380BF /* InjectedBundleRangeHandle.cpp in Sources */,
2D913441212CF9F000128AFD /* JSNPMethod.cpp in Sources */,
Modified: trunk/Source/WebKit/WebProcess/WebProcess.cpp (265714 => 265715)
--- trunk/Source/WebKit/WebProcess/WebProcess.cpp 2020-08-14 23:09:54 UTC (rev 265714)
+++ trunk/Source/WebKit/WebProcess/WebProcess.cpp 2020-08-14 23:27:26 UTC (rev 265715)
@@ -294,10 +294,6 @@
{
AuxiliaryProcess::initializeConnection(connection);
-#if PLATFORM(COCOA)
- handleXPCEndpointMessages();
-#endif
-
// We call _exit() directly from the background queue in case the main thread is unresponsive
// and AuxiliaryProcess::didClose() does not get called.
connection->setDidCloseOnConnectionWorkQueueCallback(callExit);
Modified: trunk/Source/WebKit/WebProcess/WebProcess.h (265714 => 265715)
--- trunk/Source/WebKit/WebProcess/WebProcess.h 2020-08-14 23:09:54 UTC (rev 265714)
+++ trunk/Source/WebKit/WebProcess/WebProcess.h 2020-08-14 23:27:26 UTC (rev 265715)
@@ -523,10 +523,6 @@
bool isAlwaysOnLoggingAllowed() { return m_sessionID ? m_sessionID->isAlwaysOnLoggingAllowed() : true; }
-#if PLATFORM(COCOA)
- void handleXPCEndpointMessages() const;
-#endif
-
RefPtr<WebConnectionToUIProcess> m_webConnection;
HashMap<WebCore::PageIdentifier, RefPtr<WebPage>> m_pageMap;
Added: trunk/Source/WebKit/WebProcess/cocoa/HandleXPCEndpointMessages.h (0 => 265715)
--- trunk/Source/WebKit/WebProcess/cocoa/HandleXPCEndpointMessages.h (rev 0)
+++ trunk/Source/WebKit/WebProcess/cocoa/HandleXPCEndpointMessages.h 2020-08-14 23:27:26 UTC (rev 265715)
@@ -0,0 +1,34 @@
+/*
+ * Copyright (C) 2020 Apple Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS
+ * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
+ * THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#pragma once
+
+#include <wtf/spi/darwin/XPCSPI.h>
+
+namespace WebKit {
+
+void handleXPCEndpointMessages(xpc_object_t event);
+
+}
Added: trunk/Source/WebKit/WebProcess/cocoa/HandleXPCEndpointMessages.mm (0 => 265715)
--- trunk/Source/WebKit/WebProcess/cocoa/HandleXPCEndpointMessages.mm (rev 0)
+++ trunk/Source/WebKit/WebProcess/cocoa/HandleXPCEndpointMessages.mm 2020-08-14 23:27:26 UTC (rev 265715)
@@ -0,0 +1,55 @@
+/*
+ * Copyright (C) 2020 Apple Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS
+ * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
+ * THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#import "config.h"
+#import "HandleXPCEndpointMessages.h"
+
+#import "LaunchServicesDatabaseManager.h"
+#import "LaunchServicesDatabaseXPCConstants.h"
+#import "XPCEndpoint.h"
+
+#import <wtf/text/WTFString.h>
+
+namespace WebKit {
+
+void handleXPCEndpointMessages(xpc_object_t event)
+{
+ if (xpc_get_type(event) != XPC_TYPE_DICTIONARY)
+ return;
+
+#if HAVE(LSDATABASECONTEXT)
+ String messageName = xpc_dictionary_get_string(event, XPCEndpoint::xpcMessageNameKey);
+ if (messageName.isEmpty())
+ return;
+
+ if (messageName == LaunchServicesDatabaseXPCConstants::xpcLaunchServicesDatabaseXPCEndpointMessageName) {
+ auto endpoint = xpc_dictionary_get_value(event, LaunchServicesDatabaseXPCConstants::xpcLaunchServicesDatabaseXPCEndpointNameKey);
+ LaunchServicesDatabaseManager::singleton().setEndpoint(endpoint);
+ return;
+ }
+#endif
+}
+
+}
Modified: trunk/Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm (265714 => 265715)
--- trunk/Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm 2020-08-14 23:09:54 UTC (rev 265714)
+++ trunk/Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm 2020-08-14 23:27:26 UTC (rev 265715)
@@ -26,8 +26,6 @@
#import "config.h"
#import "WebProcess.h"
-#import "LaunchServicesDatabaseManager.h"
-#import "LaunchServicesDatabaseXPCConstants.h"
#import "LegacyCustomProtocolManager.h"
#import "LogInitialization.h"
#import "Logging.h"
@@ -49,7 +47,6 @@
#import "WebProcessProxyMessages.h"
#import "WebSleepDisablerClient.h"
#import "WebsiteDataStoreParameters.h"
-#import "XPCEndpoint.h"
#import <_javascript_Core/ConfigFile.h>
#import <_javascript_Core/Options.h>
#import <WebCore/AVAssetMIMETypeCache.h>
@@ -167,41 +164,6 @@
}
#endif
-void WebProcess::handleXPCEndpointMessages() const
-{
- if (!parentProcessConnection())
- return;
-
- auto connection = parentProcessConnection()->xpcConnection();
-
- if (!connection)
- return;
-
- RELEASE_ASSERT(xpc_get_type(connection) == XPC_TYPE_CONNECTION);
-
- xpc_connection_suspend(connection);
-
- xpc_connection_set_target_queue(connection, dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0));
- xpc_connection_set_event_handler(connection, ^(xpc_object_t event) {
- if (xpc_get_type(event) != XPC_TYPE_DICTIONARY)
- return;
-
- String messageName = xpc_dictionary_get_string(event, XPCEndpoint::xpcMessageNameKey);
- if (messageName.isEmpty())
- return;
-
-#if HAVE(LSDATABASECONTEXT)
- if (messageName == LaunchServicesDatabaseXPCConstants::xpcLaunchServicesDatabaseXPCEndpointMessageName) {
- auto endpoint = xpc_dictionary_get_value(event, LaunchServicesDatabaseXPCConstants::xpcLaunchServicesDatabaseXPCEndpointNameKey);
- LaunchServicesDatabaseManager::singleton().setEndpoint(endpoint);
- return;
- }
-#endif
- });
-
- xpc_connection_resume(connection);
-}
-
void WebProcess::platformInitializeWebProcess(WebProcessCreationParameters& parameters)
{
if (parameters.mobileGestaltExtensionHandle) {