Changeset 204414 in webkit


Ignore:
Timestamp:
Aug 12, 2016, 11:50:02 AM (9 years ago)
Author:
andersca@apple.com
Message:

message loading never finishes in Mail
​https://bugs.webkit.org/show_bug.cgi?id=160806
rdar://problem/27624095

Reviewed by Dan Bernstein.

Add more checks for when a process goes away before we've established a proper connection to it.

  • Platform/IPC/mac/ConnectionMac.mm:

(IPC::Connection::receiveSourceEventHandler):
Handle the MACH_NOTIFY_NO_SENDERS and MACH_NOTIFY_SEND_ONCE messages here. Also, once we receive a send
right from the other side, stop listening for the MACH_NOTIFY_NO_SENDERS notification.

  • UIProcess/ChildProcessProxy.cpp:

(WebKit::ChildProcessProxy::didFinishLaunching):
Null check the connection identifier.

  • UIProcess/Launcher/ProcessLauncher.cpp:

(WebKit::ProcessLauncher::ProcessLauncher):
(WebKit::processLauncherWorkQueue): Deleted.
Get rid of the process launcher queue - we're not doing any blocking work here.

  • UIProcess/Launcher/ProcessLauncher.h:

Add a weak factory.

  • UIProcess/Launcher/mac/ProcessLauncherMac.mm:

(WebKit::systemDirectoryPath):
Move this before launchProcess().

(WebKit::ProcessLauncher::launchProcess):
Merge createService and connectToService into launchProcess. Also make the following changes:

  • Use mach_port_request_notification to get a notification for when our receive right loses all its senders.

This lets us listen for the other process going away before we have a send right for it.

  • Use xpc_connection_set_event_handler to listen for errors, so we can detect the process going away before

we've sent a message to it.

(WebKit::connectToService): Deleted.
(WebKit::createService): Deleted.

  • UIProcess/Network/NetworkProcessProxy.cpp:

(WebKit::NetworkProcessProxy::didFinishLaunching):
If we failed to launch, call networkProcessCrashedOrFailedToLaunch so we'll unblock any waiting web processes.

  • UIProcess/WebProcessProxy.cpp:

(WebKit::WebProcessProxy::didFinishLaunching):
Null check the connection and XPC connection before trying to get its pid.

Location:
trunk/Source/WebKit2
Files:
8 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebKit2/ChangeLog

    r204401 r204414  
     12016-08-12  Anders Carlsson  <andersca@apple.com>
     2
     3        message loading never finishes in Mail
     4        https://bugs.webkit.org/show_bug.cgi?id=160806
     5        rdar://problem/27624095
     6
     7        Reviewed by Dan Bernstein.
     8
     9        Add more checks for when a process goes away before we've established a proper connection to it.
     10
     11        * Platform/IPC/mac/ConnectionMac.mm:
     12        (IPC::Connection::receiveSourceEventHandler):
     13        Handle the MACH_NOTIFY_NO_SENDERS and MACH_NOTIFY_SEND_ONCE messages here. Also, once we receive a send
     14        right from the other side, stop listening for the MACH_NOTIFY_NO_SENDERS notification.
     15
     16        * UIProcess/ChildProcessProxy.cpp:
     17        (WebKit::ChildProcessProxy::didFinishLaunching):
     18        Null check the connection identifier.
     19
     20        * UIProcess/Launcher/ProcessLauncher.cpp:
     21        (WebKit::ProcessLauncher::ProcessLauncher):
     22        (WebKit::processLauncherWorkQueue): Deleted.
     23        Get rid of the process launcher queue - we're not doing any blocking work here.
     24
     25        * UIProcess/Launcher/ProcessLauncher.h:
     26        Add a weak factory.
     27
     28        * UIProcess/Launcher/mac/ProcessLauncherMac.mm:
     29        (WebKit::systemDirectoryPath):
     30        Move this before launchProcess().
     31
     32        (WebKit::ProcessLauncher::launchProcess):
     33        Merge createService and connectToService into launchProcess. Also make the following changes:
     34        - Use mach_port_request_notification to get a notification for when our receive right loses all its senders.
     35        This lets us listen for the other process going away before we have a send right for it.
     36        - Use xpc_connection_set_event_handler to listen for errors, so we can detect the process going away before
     37        we've sent a message to it.
     38       
     39        (WebKit::connectToService): Deleted.
     40        (WebKit::createService): Deleted.
     41
     42        * UIProcess/Network/NetworkProcessProxy.cpp:
     43        (WebKit::NetworkProcessProxy::didFinishLaunching):
     44        If we failed to launch, call networkProcessCrashedOrFailedToLaunch so we'll unblock any waiting web processes.
     45
     46        * UIProcess/WebProcessProxy.cpp:
     47        (WebKit::WebProcessProxy::didFinishLaunching):
     48        Null check the connection and XPC connection before trying to get its pid.
     49
    1502016-08-11  Brady Eidson  <beidson@apple.com>
    251

  • trunk/Source/WebKit2/Platform/IPC/mac/ConnectionMac.mm

    r203387 r204414  
    480480        return;
    481481
     482    switch (header->msgh_id) {
     483    case MACH_NOTIFY_NO_SENDERS:
     484        ASSERT(m_isServer);
     485        if (!m_sendPort)
     486            connectionDidClose();
     487        return;
     488
     489    case MACH_NOTIFY_SEND_ONCE:
     490        return;
     491
     492    default:
     493        break;
     494    }
     495
    482496    std::unique_ptr<MessageDecoder> decoder = createMessageDecoder(header);
    483497    ASSERT(decoder);
    … …  
    501515       
    502516        if (m_sendPort) {
     517            mach_port_t previousNotificationPort;
     518            mach_port_request_notification(mach_task_self(), m_receivePort, MACH_NOTIFY_NO_SENDERS, 0, MACH_PORT_NULL, MACH_MSG_TYPE_MOVE_SEND_ONCE, &previousNotificationPort);
     519
     520            if (previousNotificationPort != MACH_PORT_NULL)
     521                mach_port_deallocate(mach_task_self(), previousNotificationPort);
     522
    503523            initializeDeadNameSource();
    504524            dispatch_resume(m_deadNameSource);

  • trunk/Source/WebKit2/UIProcess/ChildProcessProxy.cpp

    r199002 r204414  
    172172    ASSERT(!m_connection);
    173173
     174    if (IPC::Connection::identifierIsNull(connectionIdentifier))
     175        return;
     176
    174177    m_connection = IPC::Connection::createServerConnection(connectionIdentifier, *this);
    175178#if PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED <= 101000

  • trunk/Source/WebKit2/UIProcess/Launcher/ProcessLauncher.cpp

    r203303 r204414  
    3232namespace WebKit {
    3333
    34 static WorkQueue& processLauncherWorkQueue()
    35 {
    36 
    37     static WorkQueue& processLauncherWorkQueue = WorkQueue::create("com.apple.WebKit.ProcessLauncher").leakRef();
    38     return processLauncherWorkQueue;
    39 }
    40 
    4134ProcessLauncher::ProcessLauncher(Client* client, const LaunchOptions& launchOptions)
    4235    : m_client(client)
     36    , m_weakPtrFactory(this)
    4337    , m_launchOptions(launchOptions)
    4438    , m_processIdentifier(0)
    4539{
    4640    m_isLaunching = true;
    47 
    48     processLauncherWorkQueue().dispatch([processLauncher = makeRef(*this)]() mutable {
    49         processLauncher->launchProcess();
    50     });
     41    launchProcess();
    5142}
    5243

  • trunk/Source/WebKit2/UIProcess/Launcher/ProcessLauncher.h

    r196661 r204414  
    2424 */
    2525
    26 #ifndef WebProcessLauncher_h
    27 #define WebProcessLauncher_h
     26#pragma once
    2827
    2928#include "Connection.h"
    … …  
    3130#include <wtf/RefPtr.h>
    3231#include <wtf/Threading.h>
     32#include <wtf/WeakPtr.h>
    3333#include <wtf/text/StringHash.h>
    3434#include <wtf/text/WTFString.h>
    … …  
    8787    Client* m_client;
    8888
     89    WeakPtrFactory<ProcessLauncher> m_weakPtrFactory;
    8990    const LaunchOptions m_launchOptions;
    9091    bool m_isLaunching;
    … …  
    9394
    9495} // namespace WebKit
    95 
    96 #endif // WebProcessLauncher_h

  • trunk/Source/WebKit2/UIProcess/Launcher/mac/ProcessLauncherMac.mm

    r201575 r204414  
    5151namespace WebKit {
    5252
    53 typedef void (ProcessLauncher::*DidFinishLaunchingProcessFunction)(pid_t, IPC::Connection::Identifier);
    54 
    5553static const char* serviceName(const ProcessLauncher::LaunchOptions& launchOptions)
    5654{
    … …  
    8482#endif
    8583}
    86    
    87 static void connectToService(const ProcessLauncher::LaunchOptions& launchOptions, bool forDevelopment, ProcessLauncher* that, DidFinishLaunchingProcessFunction didFinishLaunchingProcessFunction)
    88 {
    89     // Create a connection to the WebKit XPC service.
    90     auto connection = adoptOSObject(xpc_connection_create(serviceName(launchOptions), 0));
     84
     85static NSString *systemDirectoryPath()
     86{
     87    static NSString *path = [^{
     88#if PLATFORM(IOS_SIMULATOR)
     89        char *simulatorRoot = getenv("SIMULATOR_ROOT");
     90        return simulatorRoot ? [NSString stringWithFormat:@"%s/System/", simulatorRoot] : @"/System/";
     91#else
     92        return @"/System/";
     93#endif
     94    }() copy];
     95
     96    return path;
     97}
     98
     99void ProcessLauncher::launchProcess()
     100{
     101    auto connection = adoptOSObject(xpc_connection_create(serviceName(m_launchOptions), dispatch_get_main_queue()));
    91102
    92103    uuid_t uuid;
    … …  
    112123#endif
    113124
    114     auto languagesIterator = launchOptions.extraInitializationData.find("OverrideLanguages");
    115     if (languagesIterator != launchOptions.extraInitializationData.end()) {
     125    auto languagesIterator = m_launchOptions.extraInitializationData.find("OverrideLanguages");
     126    if (languagesIterator != m_launchOptions.extraInitializationData.end()) {
    116127        auto languages = adoptOSObject(xpc_array_create(nullptr, 0));
    117128        Vector<String> languageVector;
    … …  
    124135    xpc_connection_set_bootstrap(connection.get(), initializationMessage.get());
    125136
    126     // XPC requires having an event handler, even if it is not used.
    127     xpc_connection_set_event_handler(connection.get(), ^(xpc_object_t event) { });
    128     xpc_connection_resume(connection.get());
    129    
    130     if (shouldLeakBoost(launchOptions)) {
     137    if (shouldLeakBoost(m_launchOptions)) {
    131138        auto preBootstrapMessage = adoptOSObject(xpc_dictionary_create(nullptr, nullptr, 0));
    132139        xpc_dictionary_set_string(preBootstrapMessage.get(), "message-name", "pre-bootstrap");
    … …  
    141148    mach_port_insert_right(mach_task_self(), listeningPort, listeningPort, MACH_MSG_TYPE_MAKE_SEND);
    142149
     150    mach_port_t previousNotificationPort;
     151    mach_port_request_notification(mach_task_self(), listeningPort, MACH_NOTIFY_NO_SENDERS, 0, listeningPort, MACH_MSG_TYPE_MAKE_SEND_ONCE, &previousNotificationPort);
     152    ASSERT(!previousNotificationPort);
     153
    143154    String clientIdentifier;
    144155#if PLATFORM(MAC)
    … …  
    158169    xpc_dictionary_set_string(bootstrapMessage.get(), "ui-process-name", [[[NSProcessInfo processInfo] processName] UTF8String]);
    159170
    160     if (forDevelopment) {
     171    bool isWebKitDevelopmentBuild = ![[[[NSBundle bundleWithIdentifier:@"com.apple.WebKit"] bundlePath] stringByDeletingLastPathComponent] hasPrefix:systemDirectoryPath()];
     172    if (isWebKitDevelopmentBuild) {
    161173        xpc_dictionary_set_fd(bootstrapMessage.get(), "stdout", STDOUT_FILENO);
    162174        xpc_dictionary_set_fd(bootstrapMessage.get(), "stderr", STDERR_FILENO);
    … …  
    165177    auto extraInitializationData = adoptOSObject(xpc_dictionary_create(nullptr, nullptr, 0));
    166178
    167     for (const auto& keyValuePair : launchOptions.extraInitializationData)
     179    for (const auto& keyValuePair : m_launchOptions.extraInitializationData)
    168180        xpc_dictionary_set_string(extraInitializationData.get(), keyValuePair.key.utf8().data(), keyValuePair.value.utf8().data());
    169181
    170182    xpc_dictionary_set_value(bootstrapMessage.get(), "extra-initialization-data", extraInitializationData.get());
    171183
    172     that->ref();
    173 
    174     xpc_connection_send_message_with_reply(connection.get(), bootstrapMessage.get(), dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^(xpc_object_t reply) {
    175         xpc_type_t type = xpc_get_type(reply);
    176         if (type == XPC_TYPE_ERROR) {
    177             // We failed to launch. Release the send right.
    178             mach_port_deallocate(mach_task_self(), listeningPort);
    179 
    180             // And the receive right.
    181             mach_port_mod_refs(mach_task_self(), listeningPort, MACH_PORT_RIGHT_RECEIVE, -1);
    182 
    183             RunLoop::main().dispatch([protectedThat = RefPtr<ProcessLauncher>(that), didFinishLaunchingProcessFunction]() mutable {
    184                 (*protectedThat.*didFinishLaunchingProcessFunction)(0, IPC::Connection::Identifier());
    185             });
    186         } else {
    187             ASSERT(type == XPC_TYPE_DICTIONARY);
     184    auto weakProcessLauncher = m_weakPtrFactory.createWeakPtr();
     185    xpc_connection_set_event_handler(connection.get(), [weakProcessLauncher, listeningPort](xpc_object_t event) {
     186        ASSERT(xpc_get_type(event) == XPC_TYPE_ERROR);
     187
     188        auto processLauncher = weakProcessLauncher.get();
     189        if (!processLauncher)
     190            return;
     191
     192        if (!processLauncher->isLaunching())
     193            return;
     194
     195        // We failed to launch. Release the send right.
     196        mach_port_deallocate(mach_task_self(), listeningPort);
     197
     198        // And the receive right.
     199        mach_port_mod_refs(mach_task_self(), listeningPort, MACH_PORT_RIGHT_RECEIVE, -1);
     200        processLauncher->didFinishLaunchingProcess(0, IPC::Connection::Identifier());
     201    });
     202
     203    xpc_connection_resume(connection.get());
     204
     205    ref();
     206    xpc_connection_send_message_with_reply(connection.get(), bootstrapMessage.get(), dispatch_get_main_queue(), ^(xpc_object_t reply) {
     207        // Errors are handled in the event handler.
     208        if (xpc_get_type(reply) != XPC_TYPE_ERROR) {
     209            ASSERT(xpc_get_type(reply) == XPC_TYPE_DICTIONARY);
    188210            ASSERT(!strcmp(xpc_dictionary_get_string(reply, "message-name"), "process-finished-launching"));
    189211
    … …  
    191213            pid_t processIdentifier = xpc_connection_get_pid(connection.get());
    192214
    193             // We've finished launching the process, message back to the main run loop. This takes ownership of the connection.
    194             RunLoop::main().dispatch([protectedThat = RefPtr<ProcessLauncher>(that), didFinishLaunchingProcessFunction, processIdentifier, listeningPort, connection] {
    195                 (*protectedThat.*didFinishLaunchingProcessFunction)(processIdentifier, IPC::Connection::Identifier(listeningPort, connection));
    196             });
     215            didFinishLaunchingProcess(processIdentifier, IPC::Connection::Identifier(listeningPort, connection));
    197216        }
    198217
    199         that->deref();
     218        deref();
    200219    });
    201 }
    202 
    203 static void createService(const ProcessLauncher::LaunchOptions& launchOptions, bool forDevelopment, ProcessLauncher* that, DidFinishLaunchingProcessFunction didFinishLaunchingProcessFunction)
    204 {
    205     connectToService(launchOptions, forDevelopment, that, didFinishLaunchingProcessFunction);
    206 }
    207 
    208 static NSString *systemDirectoryPath()
    209 {
    210     static NSString *path = [^{
    211 #if PLATFORM(IOS_SIMULATOR)
    212         char *simulatorRoot = getenv("SIMULATOR_ROOT");
    213         return simulatorRoot ? [NSString stringWithFormat:@"%s/System/", simulatorRoot] : @"/System/";
    214 #else
    215         return @"/System/";
    216 #endif
    217     }() copy];
    218 
    219     return path;
    220 }
    221 
    222 void ProcessLauncher::launchProcess()
    223 {
    224     bool isWebKitDevelopmentBuild = ![[[[NSBundle bundleWithIdentifier:@"com.apple.WebKit"] bundlePath] stringByDeletingLastPathComponent] hasPrefix:systemDirectoryPath()];
    225 
    226     createService(m_launchOptions, isWebKitDevelopmentBuild, this, &ProcessLauncher::didFinishLaunchingProcess);
    227220}
    228221

  • trunk/Source/WebKit2/UIProcess/Network/NetworkProcessProxy.cpp

    r203856 r204414  
    300300
    301301    if (IPC::Connection::identifierIsNull(connectionIdentifier)) {
    302         // FIXME: Do better cleanup here.
     302        networkProcessCrashedOrFailedToLaunch();
    303303        return;
    304304    }

  • trunk/Source/WebKit2/UIProcess/WebProcessProxy.cpp

    r204243 r204414  
    614614
    615615#if PLATFORM(IOS)
    616     xpc_connection_t xpcConnection = connection()->xpcConnection();
    617     ASSERT(xpcConnection);
    618     m_throttler.didConnectToProcess(xpc_connection_get_pid(xpcConnection));
     616    if (connection()) {
     617        if (xpc_connection_t xpcConnection = connection()->xpcConnection())
     618            m_throttler.didConnectToProcess(xpc_connection_get_pid(xpcConnection));
     619    }
    619620#endif
    620621}
Note: See TracChangeset for help on using the changeset viewer.