Title: [230084] trunk/Source/WebKit
Revision
230084
Author
bfulg...@apple.com
Date
2018-03-29 14:59:54 -0700 (Thu, 29 Mar 2018)

Log Message

REGRESSION(r230035): ASSERT(MACH_PORT_VALID(m_sendPort)) hit in IPC::Connection::initializeSendSource()
https://bugs.webkit.org/show_bug.cgi?id=184122
<rdar://problem/39003606>

Reviewed by Chris Dumez.

One of the new assertions added in r230035 begin firing while running tests locally. This was happening
because the WebInspector was attempting to open a new connection to a web process that had already
terminated its mach port connection (a dead port).
        
We should avoid opening new connections when the port we were given is already dead.

* Platform/IPC/Connection.h:
(IPC::Connection::identifierIsValid): Added.
* Platform/IPC/mac/ConnectionMac.mm:
(IPC::Connection::platformInitialize): Do not perform initialization on a dead (or null) port.
(IPC::Connection::open): Add some assertions that ports are in a valid state.
(IPC::Connection::sendOutgoingMessage): Assert that the send port is not dead.
(IPC::Connection::receiveSourceEventHandler): Assert that the receive port is valid.
* UIProcess/ChildProcessProxy.cpp:
(WebKit::ChildProcessProxy::didFinishLaunching): Treat a dead port as a signal that the
child process failed to launch.
* UIProcess/Network/NetworkProcessProxy.cpp:
(WebKit::NetworkProcessProxy::didFinishLaunching): Ditto.
* UIProcess/Plugins/PluginProcessProxy.cpp:
(WebKit::PluginProcessProxy::didFinishLaunching): Ditto.
* UIProcess/Storage/StorageProcessProxy.cpp:
(WebKit::StorageProcessProxy::didFinishLaunching): Ditto.
* WebProcess/Plugins/PluginProcessConnectionManager.cpp:
(WebKit::PluginProcessConnectionManager::getPluginProcessConnection): Ditto.
* WebProcess/WebPage/WebInspectorUI.cpp:
(WebKit::WebInspectorUI::establishConnection): Ditto.
* WebProcess/WebProcess.cpp:
(WebKit::WebProcess::ensureNetworkProcessConnection): Ditto.
(WebKit::WebProcess::ensureWebToStorageProcessConnection): Ditto.

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (230083 => 230084)


--- trunk/Source/WebKit/ChangeLog	2018-03-29 21:53:35 UTC (rev 230083)
+++ trunk/Source/WebKit/ChangeLog	2018-03-29 21:59:54 UTC (rev 230084)
@@ -1,3 +1,41 @@
+2018-03-29  Brent Fulgham  <bfulg...@apple.com>
+
+        REGRESSION(r230035): ASSERT(MACH_PORT_VALID(m_sendPort)) hit in IPC::Connection::initializeSendSource()
+        https://bugs.webkit.org/show_bug.cgi?id=184122
+        <rdar://problem/39003606>
+
+        Reviewed by Chris Dumez.
+
+        One of the new assertions added in r230035 begin firing while running tests locally. This was happening
+        because the WebInspector was attempting to open a new connection to a web process that had already
+        terminated its mach port connection (a dead port).
+        
+        We should avoid opening new connections when the port we were given is already dead.
+
+        * Platform/IPC/Connection.h:
+        (IPC::Connection::identifierIsValid): Added.
+        * Platform/IPC/mac/ConnectionMac.mm:
+        (IPC::Connection::platformInitialize): Do not perform initialization on a dead (or null) port.
+        (IPC::Connection::open): Add some assertions that ports are in a valid state.
+        (IPC::Connection::sendOutgoingMessage): Assert that the send port is not dead.
+        (IPC::Connection::receiveSourceEventHandler): Assert that the receive port is valid.
+        * UIProcess/ChildProcessProxy.cpp:
+        (WebKit::ChildProcessProxy::didFinishLaunching): Treat a dead port as a signal that the
+        child process failed to launch.
+        * UIProcess/Network/NetworkProcessProxy.cpp:
+        (WebKit::NetworkProcessProxy::didFinishLaunching): Ditto.
+        * UIProcess/Plugins/PluginProcessProxy.cpp:
+        (WebKit::PluginProcessProxy::didFinishLaunching): Ditto.
+        * UIProcess/Storage/StorageProcessProxy.cpp:
+        (WebKit::StorageProcessProxy::didFinishLaunching): Ditto.
+        * WebProcess/Plugins/PluginProcessConnectionManager.cpp:
+        (WebKit::PluginProcessConnectionManager::getPluginProcessConnection): Ditto.
+        * WebProcess/WebPage/WebInspectorUI.cpp:
+        (WebKit::WebInspectorUI::establishConnection): Ditto.
+        * WebProcess/WebProcess.cpp:
+        (WebKit::WebProcess::ensureNetworkProcessConnection): Ditto.
+        (WebKit::WebProcess::ensureWebToStorageProcessConnection): Ditto.
+
 2018-03-29  Youenn Fablet  <you...@apple.com>
 
         Synchronize SecurityOrigin related scheme registries with NetworkProcess

Modified: trunk/Source/WebKit/Platform/IPC/Connection.h (230083 => 230084)


--- trunk/Source/WebKit/Platform/IPC/Connection.h	2018-03-29 21:53:35 UTC (rev 230083)
+++ trunk/Source/WebKit/Platform/IPC/Connection.h	2018-03-29 21:59:54 UTC (rev 230084)
@@ -100,7 +100,7 @@
 
 #if USE(UNIX_DOMAIN_SOCKETS)
     typedef int Identifier;
-    static bool identifierIsNull(Identifier identifier) { return identifier == -1; }
+    static bool identifierIsValid(Identifier identifier) { return identifier != -1; }
 
     struct SocketPair {
         int client;
@@ -133,7 +133,7 @@
         mach_port_t port { MACH_PORT_NULL };
         OSObjectPtr<xpc_connection_t> xpcConnection;
     };
-    static bool identifierIsNull(Identifier identifier) { return identifier.port == MACH_PORT_NULL; }
+    static bool identifierIsValid(Identifier identifier) { return MACH_PORT_VALID(identifier.port); }
     xpc_connection_t xpcConnection() const { return m_xpcConnection.get(); }
     bool getAuditToken(audit_token_t&);
     pid_t remoteProcessID() const;
@@ -140,7 +140,7 @@
 #elif OS(WINDOWS)
     typedef HANDLE Identifier;
     static bool createServerAndClientIdentifiers(Identifier& serverIdentifier, Identifier& clientIdentifier);
-    static bool identifierIsNull(Identifier identifier) { return !identifier; }
+    static bool identifierIsValid(Identifier identifier) { return !!identifier; }
 #endif
 
     static Ref<Connection> createServerConnection(Identifier, Client&);

Modified: trunk/Source/WebKit/Platform/IPC/mac/ConnectionMac.mm (230083 => 230084)


--- trunk/Source/WebKit/Platform/IPC/mac/ConnectionMac.mm	2018-03-29 21:53:35 UTC (rev 230083)
+++ trunk/Source/WebKit/Platform/IPC/mac/ConnectionMac.mm	2018-03-29 21:59:54 UTC (rev 230084)
@@ -156,6 +156,9 @@
     
 void Connection::platformInitialize(Identifier identifier)
 {
+    if (!MACH_PORT_VALID(identifier.port))
+        return;
+
     if (m_isServer) {
         m_receivePort = identifier.port;
         m_sendPort = MACH_PORT_NULL;
@@ -179,10 +182,11 @@
     if (m_isServer) {
         ASSERT(m_receivePort);
         ASSERT(!m_sendPort);
-        
+        ASSERT(MACH_PORT_VALID(m_receivePort));
     } else {
         ASSERT(!m_receivePort);
         ASSERT(m_sendPort);
+        ASSERT(MACH_PORT_VALID(m_sendPort));
 
         auto kr = mach_port_allocate(mach_task_self(), MACH_PORT_RIGHT_RECEIVE, &m_receivePort);
         if (kr != KERN_SUCCESS) {
@@ -358,6 +362,7 @@
         memcpy(messageData, encoder->buffer(), encoder->bufferSize());
 
     ASSERT(m_sendPort);
+    ASSERT(MACH_PORT_VALID(m_sendPort));
 
     return sendMessage(WTFMove(message));
 }
@@ -493,6 +498,7 @@
 {
     ReceiveBuffer buffer;
 
+    ASSERT(MACH_PORT_VALID(m_receivePort));
     mach_msg_header_t* header = readFromMachPort(m_receivePort, buffer);
     if (!header)
         return;
@@ -532,6 +538,7 @@
         m_sendPort = port.port();
         
         if (m_sendPort) {
+            ASSERT(MACH_PORT_VALID(m_receivePort));
             mach_port_t previousNotificationPort = MACH_PORT_NULL;
             auto kr = mach_port_request_notification(mach_task_self(), m_receivePort, MACH_NOTIFY_NO_SENDERS, 0, MACH_PORT_NULL, MACH_MSG_TYPE_MOVE_SEND_ONCE, &previousNotificationPort);
             ASSERT(kr == KERN_SUCCESS);

Modified: trunk/Source/WebKit/UIProcess/ChildProcessProxy.cpp (230083 => 230084)


--- trunk/Source/WebKit/UIProcess/ChildProcessProxy.cpp	2018-03-29 21:53:35 UTC (rev 230083)
+++ trunk/Source/WebKit/UIProcess/ChildProcessProxy.cpp	2018-03-29 21:59:54 UTC (rev 230084)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2012 Apple Inc. All rights reserved.
+ * Copyright (C) 2012-2018 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -165,7 +165,7 @@
 {
     ASSERT(!m_connection);
 
-    if (IPC::Connection::identifierIsNull(connectionIdentifier))
+    if (!IPC::Connection::identifierIsValid(connectionIdentifier))
         return;
 
     m_connection = IPC::Connection::createServerConnection(connectionIdentifier, *this);

Modified: trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp (230083 => 230084)


--- trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp	2018-03-29 21:53:35 UTC (rev 230083)
+++ trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp	2018-03-29 21:59:54 UTC (rev 230084)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2012-2016 Apple Inc. All rights reserved.
+ * Copyright (C) 2012-2018 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -327,7 +327,7 @@
 {
     ChildProcessProxy::didFinishLaunching(launcher, connectionIdentifier);
 
-    if (IPC::Connection::identifierIsNull(connectionIdentifier)) {
+    if (!IPC::Connection::identifierIsValid(connectionIdentifier)) {
         networkProcessFailedToLaunch();
         return;
     }

Modified: trunk/Source/WebKit/UIProcess/Plugins/PluginProcessProxy.cpp (230083 => 230084)


--- trunk/Source/WebKit/UIProcess/Plugins/PluginProcessProxy.cpp	2018-03-29 21:53:35 UTC (rev 230083)
+++ trunk/Source/WebKit/UIProcess/Plugins/PluginProcessProxy.cpp	2018-03-29 21:59:54 UTC (rev 230084)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2010 Apple Inc. All rights reserved.
+ * Copyright (C) 2010-2018 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -215,7 +215,7 @@
 {
     ASSERT(!m_connection);
 
-    if (IPC::Connection::identifierIsNull(connectionIdentifier)) {
+    if (!IPC::Connection::identifierIsValid(connectionIdentifier)) {
         pluginProcessCrashedOrFailedToLaunch();
         return;
     }

Modified: trunk/Source/WebKit/UIProcess/Storage/StorageProcessProxy.cpp (230083 => 230084)


--- trunk/Source/WebKit/UIProcess/Storage/StorageProcessProxy.cpp	2018-03-29 21:53:35 UTC (rev 230083)
+++ trunk/Source/WebKit/UIProcess/Storage/StorageProcessProxy.cpp	2018-03-29 21:59:54 UTC (rev 230084)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2013 Apple Inc. All rights reserved.
+ * Copyright (C) 2013-2018 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -219,7 +219,7 @@
 {
     ChildProcessProxy::didFinishLaunching(launcher, connectionIdentifier);
 
-    if (IPC::Connection::identifierIsNull(connectionIdentifier)) {
+    if (!IPC::Connection::identifierIsValid(connectionIdentifier)) {
         // FIXME: Do better cleanup here.
         return;
     }

Modified: trunk/Source/WebKit/WebProcess/Plugins/PluginProcessConnectionManager.cpp (230083 => 230084)


--- trunk/Source/WebKit/WebProcess/Plugins/PluginProcessConnectionManager.cpp	2018-03-29 21:53:35 UTC (rev 230083)
+++ trunk/Source/WebKit/WebProcess/Plugins/PluginProcessConnectionManager.cpp	2018-03-29 21:59:54 UTC (rev 230084)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2010 Apple Inc. All rights reserved.
+ * Copyright (C) 2010-2018 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -72,7 +72,7 @@
     bool supportsAsynchronousInitialization;
     if (!WebProcess::singleton().parentProcessConnection()->sendSync(Messages::WebProcessProxy::GetPluginProcessConnection(pluginProcessToken),
                                                      Messages::WebProcessProxy::GetPluginProcessConnection::Reply(encodedConnectionIdentifier, supportsAsynchronousInitialization), 0))
-        return 0;
+        return nullptr;
 
 #if USE(UNIX_DOMAIN_SOCKETS)
     IPC::Connection::Identifier connectionIdentifier = encodedConnectionIdentifier.releaseFileDescriptor();
@@ -81,7 +81,7 @@
 #elif OS(WINDOWS)
     IPC::Connection::Identifier connectionIdentifier = encodedConnectionIdentifier.handle();
 #endif
-    if (IPC::Connection::identifierIsNull(connectionIdentifier))
+    if (!IPC::Connection::identifierIsValid(connectionIdentifier))
         return nullptr;
 
     RefPtr<PluginProcessConnection> pluginProcessConnection = PluginProcessConnection::create(this, pluginProcessToken, connectionIdentifier, supportsAsynchronousInitialization);

Modified: trunk/Source/WebKit/WebProcess/WebPage/WebInspectorUI.cpp (230083 => 230084)


--- trunk/Source/WebKit/WebProcess/WebPage/WebInspectorUI.cpp	2018-03-29 21:53:35 UTC (rev 230083)
+++ trunk/Source/WebKit/WebProcess/WebPage/WebInspectorUI.cpp	2018-03-29 21:59:54 UTC (rev 230084)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2014, 2016 Apple Inc. All rights reserved.
+ * Copyright (C) 2014-2018 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -66,7 +66,7 @@
     return;
 #endif
 
-    if (IPC::Connection::identifierIsNull(connectionIdentifier))
+    if (!IPC::Connection::identifierIsValid(connectionIdentifier))
         return;
 
     m_inspectedPageIdentifier = inspectedPageIdentifier;

Modified: trunk/Source/WebKit/WebProcess/WebProcess.cpp (230083 => 230084)


--- trunk/Source/WebKit/WebProcess/WebProcess.cpp	2018-03-29 21:53:35 UTC (rev 230083)
+++ trunk/Source/WebKit/WebProcess/WebProcess.cpp	2018-03-29 21:59:54 UTC (rev 230084)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2009-2017 Apple Inc. All rights reserved.
+ * Copyright (C) 2009-2018 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -1156,7 +1156,7 @@
 #else
         ASSERT_NOT_REACHED();
 #endif
-        if (IPC::Connection::identifierIsNull(connectionIdentifier))
+        if (!IPC::Connection::identifierIsValid(connectionIdentifier))
             CRASH();
         m_networkProcessConnection = NetworkProcessConnection::create(connectionIdentifier);
     }
@@ -1230,8 +1230,9 @@
 #else
         ASSERT_NOT_REACHED();
 #endif
-        if (IPC::Connection::identifierIsNull(connectionIdentifier))
+        if (!IPC::Connection::identifierIsValid(connectionIdentifier))
             CRASH();
+
         m_webToStorageProcessConnection = WebToStorageProcessConnection::create(connectionIdentifier);
 
     }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to