Skip to content

Commit

Permalink
Cherry-pick 3c2c899. rdar://126479653
Browse files Browse the repository at this point in the history
    WebKit process termination with xpc_connection_kill does not always work
    https://bugs.webkit.org/show_bug.cgi?id=272669
    rdar://126479653

    Reviewed by Chris Dumez.

    WebKit process termination with xpc_connection_kill does not always work. We are currently seeing flaky
    termination behavior on macOS, where the child processes are not always terminated successfully.
    Additionally, on iOS, the XPC connection has become anonymous due to migration to extensions for WebKit
    processes, and xpc_connection_kill does not support anonymous connections. This patch addresses this
    issue by creating and sending a XPC message to the child process to request termination. This has a
    high chance of success, since we know that the XPC connection termination watchdog is holding a
    background assertion on the process, so it is not suspended. Additionally, the XPC message is being
    handled on the XPC event handler thread, which is handling very few messages, so it is very unlikely
    that it is blocked and cannot handle the message. This gives the process a chance to exit cleanly and
    send a reply back. If the UI process does not receive the expected reply, it will try calling
    xpc_connection_kill.

    * Source/WebKit/Platform/cocoa/XPCUtilities.h:
    * Source/WebKit/Platform/cocoa/XPCUtilities.mm:
    (WebKit::terminateWithReason):
    (WebKit::handleXPCExitMessage):
    * Source/WebKit/Shared/Authentication/cocoa/AuthenticationManagerCocoa.mm:
    (WebKit::AuthenticationManager::initializeConnection):
    * Source/WebKit/Shared/Cocoa/XPCEndpoint.mm:
    (WebKit::XPCEndpoint::XPCEndpoint):
    * Source/WebKit/Shared/EntryPointUtilities/Cocoa/XPCService/XPCServiceMain.mm:
    (WebKit::XPCServiceEventHandler):

    Canonical link: https://commits.webkit.org/277509@main
  • Loading branch information
pvollan authored and Mohsin Qureshi committed Apr 17, 2024
1 parent c4b640f commit d7e2f94
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 0 deletions.
1 change: 1 addition & 0 deletions Source/WebKit/Platform/cocoa/XPCUtilities.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,5 +36,6 @@ enum class ReasonCode : uint64_t {
};

void terminateWithReason(xpc_connection_t, ReasonCode, const char* reason);
void handleXPCExitMessage(xpc_object_t);

}
22 changes: 22 additions & 0 deletions Source/WebKit/Platform/cocoa/XPCUtilities.mm
Original file line number Diff line number Diff line change
Expand Up @@ -26,17 +26,39 @@
#include "config.h"
#include "XPCUtilities.h"

#include <wtf/WTFProcess.h>

namespace WebKit {

static constexpr auto messageNameKey = "message-name"_s;
static constexpr auto exitProcessMessage = "exit"_s;

void terminateWithReason(xpc_connection_t connection, ReasonCode, const char*)
{
// This could use ReasonSPI.h, but currently does not as the SPI is blocked by the sandbox.
// See https://bugs.webkit.org/show_bug.cgi?id=224499 rdar://76396241
if (!connection)
return;

// Give the process a chance to exit cleanly by sending a XPC message to request termination, then try xpc_connection_kill.
auto exitMessage = adoptOSObject(xpc_dictionary_create(nullptr, nullptr, 0));
xpc_dictionary_set_string(exitMessage.get(), messageNameKey, exitProcessMessage.characters());
xpc_connection_send_message(connection, exitMessage.get());

ALLOW_DEPRECATED_DECLARATIONS_BEGIN
xpc_connection_kill(connection, SIGKILL);
ALLOW_DEPRECATED_DECLARATIONS_END
}

void handleXPCExitMessage(xpc_object_t event)
{
if (xpc_get_type(event) == XPC_TYPE_DICTIONARY) {
auto* messageName = xpc_dictionary_get_string(event, messageNameKey);
if (exitProcessMessage == messageName) {
RELEASE_LOG_ERROR(IPC, "Received exit message, exiting now.");
terminateProcess(EXIT_FAILURE);
}
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
#import "AuthenticationChallengeDisposition.h"
#import "ClientCertificateAuthenticationXPCConstants.h"
#import "Connection.h"
#import "XPCUtilities.h"
#import <pal/spi/cocoa/NSXPCConnectionSPI.h>
#import <pal/spi/cocoa/SecKeyProxySPI.h>
#import <wtf/MainThread.h>
Expand All @@ -50,6 +51,9 @@
// The following xpc event handler overwrites the boostrap event handler and is only used
// to capture client certificate credential.
xpc_connection_set_event_handler(connection->xpcConnection(), ^(xpc_object_t event) {

handleXPCExitMessage(event);

callOnMainRunLoop([event = OSObjectPtr(event), weakThis = weakThis] {
RELEASE_ASSERT(isMainRunLoop());

Expand Down
3 changes: 3 additions & 0 deletions Source/WebKit/Shared/Cocoa/XPCEndpoint.mm
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

#import "config.h"
#import "XPCEndpoint.h"
#import "XPCUtilities.h"

#import <wtf/cocoa/Entitlements.h>
#import <wtf/text/ASCIILiteral.h>
Expand All @@ -46,6 +47,8 @@
xpc_connection_set_event_handler(m_connection.get(), ^(xpc_object_t message) {
xpc_type_t type = xpc_get_type(message);

handleXPCExitMessage(message);

if (type == XPC_TYPE_CONNECTION) {
OSObjectPtr<xpc_connection_t> connection = message;
#if USE(APPLE_INTERNAL_SDK)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
#import "Logging.h"
#import "WKCrashReporter.h"
#import "XPCServiceEntryPoint.h"
#import "XPCUtilities.h"
#import <CoreFoundation/CoreFoundation.h>
#import <mach/mach.h>
#import <pal/spi/cf/CFUtilitiesSPI.h>
Expand Down Expand Up @@ -138,6 +139,8 @@ void XPCServiceEventHandler(xpc_connection_t peer)
return;
}

handleXPCExitMessage(event);

auto* messageName = xpc_dictionary_get_string(event, "message-name");
if (!messageName) {
RELEASE_LOG_ERROR(IPC, "XPCServiceEventHandler: 'message-name' is not present in the XPC dictionary");
Expand Down

0 comments on commit d7e2f94

Please sign in to comment.