Changeset 276712 in webkit

Timestamp:

Apr 28, 2021, 8:20:48 AM (5 years ago)

Author:

Chris Dumez

Message:

Crash under GPUProcessProxy::getGPUProcessConnection()
​https://bugs.webkit.org/show_bug.cgi?id=224017
<rdar://75146104>

Reviewed by Darin Adler.

The crash would happen when replyToPendingMessages() get called in the AuxiliaryProcessProxy
destructor and there would be a pending getGPUProcessConnection() IPC. The lambda in
WebProcessPool::getGPUProcessConnection() would check if a WeakPtr to the WebProcessPool is
valid before calling ensureGPUProcess().getGPUProcessConnection(). However, at this point,
the WebProcessPool destructor is still running so the WeakPtr is still valid but
ensureGPUProcess().getGPUProcessConnection() will crash because the GPUProcessProxy
destructor has already run.

• UIProcess/WebProcessPool.cpp:

(WebKit::WebProcessPool::~WebProcessPool):
Drop code that was causing the GPUProcessProxy to reply to all its pending IPC messages whenenever
a WebProcessPool gets destroyed. This is incorrect since a GPUProcessProxy may be shared by several
WebProcessPool objects. Note that the AuxiliaryProcessProxy destructor already takes care of
calling replyToPendingMessages().

(WebKit::WebProcessPool::getGPUProcessConnection):
Protect the WebProcessPool is the lambda to prevent the case where this lambda gets called and the
WebProcessPool & GPUProcessProxy are in the middle of destruction.

Location:

trunk/Source/WebKit

Files:

• ChangeLog (modified) (1 diff)
• UIProcess/WebProcessPool.cpp (modified) (2 diffs)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2021-04-28  Chris Dumez  <cdumez@apple.com>
+
+        Crash under GPUProcessProxy::getGPUProcessConnection()
+        https://bugs.webkit.org/show_bug.cgi?id=224017
+        <rdar://75146104>
+
+        Reviewed by Darin Adler.
+
+        The crash would happen when replyToPendingMessages() get called in the AuxiliaryProcessProxy
+        destructor and there would be a pending getGPUProcessConnection() IPC. The lambda in
+        WebProcessPool::getGPUProcessConnection() would check if a WeakPtr to the WebProcessPool is
+        valid before calling `ensureGPUProcess().getGPUProcessConnection()`. However, at this point, 
+        the WebProcessPool destructor is still running so the WeakPtr is still valid but 
+        `ensureGPUProcess().getGPUProcessConnection()` will crash because the GPUProcessProxy
+        destructor has already run.
+
+        * UIProcess/WebProcessPool.cpp:
+        (WebKit::WebProcessPool::~WebProcessPool):
+        Drop code that was causing the GPUProcessProxy to reply to all its pending IPC messages whenenever
+        a WebProcessPool gets destroyed. This is incorrect since a GPUProcessProxy may be shared by several
+        WebProcessPool objects. Note that the AuxiliaryProcessProxy destructor already takes care of
+        calling replyToPendingMessages().
+
+        (WebKit::WebProcessPool::getGPUProcessConnection):
+        Protect the WebProcessPool is the lambda to prevent the case where this lambda gets called and the
+        WebProcessPool & GPUProcessProxy are in the middle of destruction.
+
 2021-04-28  Kimmo Kinnunen  <kkinnunen@apple.com>
 

trunk/Source/WebKit/UIProcess/WebProcessPool.cpp

@@ -361 +361 @@
 #endif
 
-#if ENABLE(GPU_PROCESS)
-    if (m_gpuProcess)
-        m_gpuProcess->replyToPendingMessages();
-#endif
-
     // Only remaining processes should be pre-warmed ones as other keep the process pool alive.
     while (!m_processes.isEmpty()) {
@@ -530 +525 @@
 void WebProcessPool::getGPUProcessConnection(WebProcessProxy& webProcessProxy, GPUProcessConnectionParameters&& parameters, Messages::WebProcessProxy::GetGPUProcessConnection::DelayedReply&& reply)
 {
-    ensureGPUProcess().getGPUProcessConnection(webProcessProxy, parameters, [this, weakThis = makeWeakPtr(*this), parameters, webProcessProxy = makeWeakPtr(webProcessProxy), reply = WTFMove(reply)] (auto& connectionInfo) mutable {
-        if (UNLIKELY(!IPC::Connection::identifierIsValid(connectionInfo.identifier()) && webProcessProxy && weakThis)) {
+    ensureGPUProcess().getGPUProcessConnection(webProcessProxy, parameters, [this, protectedThis = makeRef(*this), parameters, webProcessProxy = makeWeakPtr(webProcessProxy), reply = WTFMove(reply)] (auto& connectionInfo) mutable {
+        if (UNLIKELY(!IPC::Connection::identifierIsValid(connectionInfo.identifier()) && webProcessProxy)) {
             WEBPROCESSPOOL_RELEASE_LOG_ERROR(Process, "getGPUProcessConnection: Failed first attempt, retrying");
             ensureGPUProcess().getGPUProcessConnection(*webProcessProxy, parameters, WTFMove(reply));