Changeset 269749 in webkit

Timestamp:

Nov 12, 2020 2:01:18 PM (3 years ago)

Author:

Chris Dumez

Message:

ASSERTION FAILED: isValidIdentifier(m_identifier) seen with TestWebKitAPI.GPUProcess.WebProcessTerminationAfterTooManyGPUProcessCrashes
​https://bugs.webkit.org/show_bug.cgi?id=218856
<rdar://problem/71331809>

Reviewed by Tim Horton.

The API test is repeatedly killing the GPU process. As a result, it is possible for the
GPUProcess to crash while RemoteAudioDestinationProxy::connectToGPUProcess() is in the
middle of its RemoteAudioDestinationManager::CreateAudioDestination() synchronous IPC.
The function would fail to check if the IPC was successful and proceed with an invalid
destinationID in such cases, causing the crash.

We now check if the sendSync() was successful. If it wasn't we now log an error and
return early. RemoteAudioDestinationManager::gpuProcessConnectionDidClose() will get
called later on to notify us that the GPU Process crashed and it will call
connectToGPUProcess() again.

No new tests, covered by existing API test that is flakily crashing.

• WebProcess/GPU/media/RemoteAudioDestinationProxy.cpp:

(WebKit::RemoteAudioDestinationProxy::connectToGPUProcess):

Location:

trunk/Source/WebKit

Files:

• ChangeLog (modified) (1 diff)
• WebProcess/GPU/media/RemoteAudioDestinationProxy.cpp (modified) (2 diffs)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2020-11-12  Chris Dumez  <cdumez@apple.com>
+
+        ASSERTION FAILED: isValidIdentifier(m_identifier) seen with TestWebKitAPI.GPUProcess.WebProcessTerminationAfterTooManyGPUProcessCrashes
+        https://bugs.webkit.org/show_bug.cgi?id=218856
+        <rdar://problem/71331809>
+
+        Reviewed by Tim Horton.
+
+        The API test is repeatedly killing the GPU process. As a result, it is possible for the
+        GPUProcess to crash while RemoteAudioDestinationProxy::connectToGPUProcess() is in the
+        middle of its RemoteAudioDestinationManager::CreateAudioDestination() synchronous IPC.
+        The function would fail to check if the IPC was successful and proceed with an invalid
+        destinationID in such cases, causing the crash.
+
+        We now check if the sendSync() was successful. If it wasn't we now log an error and
+        return early. RemoteAudioDestinationManager::gpuProcessConnectionDidClose() will get
+        called later on to notify us that the GPU Process crashed and it will call
+        connectToGPUProcess() again.
+
+        No new tests, covered by existing API test that is flakily crashing.
+
+        * WebProcess/GPU/media/RemoteAudioDestinationProxy.cpp:
+        (WebKit::RemoteAudioDestinationProxy::connectToGPUProcess):
+
 2020-11-12  Youenn Fablet  <youenn@apple.com>
 

trunk/Source/WebKit/WebProcess/GPU/media/RemoteAudioDestinationProxy.cpp

@@ -30 +30 @@
 
 #include "GPUConnectionToWebProcess.h"
+#include "Logging.h"
 #include "RemoteAudioDestinationManagerMessages.h"
 #include "RemoteAudioDestinationProxyMessages.h"
@@ -82 +83 @@
     auto& connection = WebProcess::singleton().ensureGPUProcessConnection();
     connection.addClient(*this);
-    connection.connection().sendSync(
+    bool didSucceed = connection.connection().sendSync(
         Messages::RemoteAudioDestinationManager::CreateAudioDestination(m_inputDeviceId, m_numberOfInputChannels, numberOfOutputChannels(), sampleRate(), hardwareSampleRate()),
         Messages::RemoteAudioDestinationManager::CreateAudioDestination::Reply(destinationID), 0);
+
+    if (!didSucceed) {
+        // The GPUProcess likely crashed during this synchronous IPC. gpuProcessConnectionDidClose() will get called to reconnect to the GPUProcess.
+        RELEASE_LOG_ERROR(Media, "RemoteAudioDestinationProxy::connectToGPUProcess: Failed to send RemoteAudioDestinationManager::CreateAudioDestination() IPC (GPU process likely crashed)");
+        return;
+    }
+
     connection.connection().addThreadMessageReceiver(Messages::RemoteAudioDestinationProxy::messageReceiverName(), this, destinationID.toUInt64());
-
     m_destinationID = destinationID;