Changeset 292289 in webkit

Timestamp:

Apr 4, 2022, 8:45:40 AM (4 years ago)

Author:

commit-queue@webkit.org

Message:

RemoteRenderingBackendProxy fails to maintain correct state when gpu process crashes and upon deletion
​https://bugs.webkit.org/show_bug.cgi?id=238618

Patch by Kimmo Kinnunen <​kkinnunen@apple.com> on 2022-04-04
Reviewed by Simon Fraser.

RemoteRenderingBackendProxy::m_needsWakeUpSemaphoreForDisplayListStream
was not reset when proxy would connect to a new gpu process after a crash.
RemoteRenderingBackendProxy::~RemoteRenderingBackendProxy() would not
remove the GPUConnection::Client registration.

• Platform/IPC/MessageReceiveQueueMap.cpp:

(IPC::MessageReceiveQueueMap::remove):

• Platform/IPC/StreamClientConnection.h:
• WebProcess/GPU/graphics/RemoteRenderingBackendProxy.cpp:

(WebKit::RemoteRenderingBackendProxy::~RemoteRenderingBackendProxy):
(WebKit::RemoteRenderingBackendProxy::gpuProcessConnectionDidClose):
(WebKit::RemoteRenderingBackendProxy::disconnectGPUProcess):
(WebKit::RemoteRenderingBackendProxy::streamConnection):
(WebKit::RemoteRenderingBackendProxy::didCreateWakeUpSemaphoreForDisplayListStream):

• WebProcess/GPU/graphics/RemoteRenderingBackendProxy.h:

Location:

trunk/Source/WebKit

Files:

• ChangeLog (modified) (1 diff)
• Platform/IPC/StreamClientConnection.h (modified) (1 diff)
• WebProcess/GPU/graphics/RemoteRenderingBackendProxy.cpp (modified) (5 diffs)
• WebProcess/GPU/graphics/RemoteRenderingBackendProxy.h (modified) (2 diffs)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2022-04-04  Kimmo Kinnunen  <kkinnunen@apple.com>
+
+        RemoteRenderingBackendProxy fails to maintain correct state when gpu process crashes and upon deletion
+        https://bugs.webkit.org/show_bug.cgi?id=238618
+
+        Reviewed by Simon Fraser.
+
+        RemoteRenderingBackendProxy::m_needsWakeUpSemaphoreForDisplayListStream
+        was not reset when proxy would connect to a new gpu process after a crash.
+        RemoteRenderingBackendProxy::~RemoteRenderingBackendProxy() would not
+        remove the GPUConnection::Client registration.
+
+        * Platform/IPC/MessageReceiveQueueMap.cpp:
+        (IPC::MessageReceiveQueueMap::remove):
+        * Platform/IPC/StreamClientConnection.h:
+        * WebProcess/GPU/graphics/RemoteRenderingBackendProxy.cpp:
+        (WebKit::RemoteRenderingBackendProxy::~RemoteRenderingBackendProxy):
+        (WebKit::RemoteRenderingBackendProxy::gpuProcessConnectionDidClose):
+        (WebKit::RemoteRenderingBackendProxy::disconnectGPUProcess):
+        (WebKit::RemoteRenderingBackendProxy::streamConnection):
+        (WebKit::RemoteRenderingBackendProxy::didCreateWakeUpSemaphoreForDisplayListStream):
+        * WebProcess/GPU/graphics/RemoteRenderingBackendProxy.h:
+
 2022-04-04  Kimmo Kinnunen  <kkinnunen@apple.com>
 

trunk/Source/WebKit/Platform/IPC/StreamClientConnection.h

@@ -62 +62 @@
     StreamConnectionBuffer& streamBuffer() { return m_buffer; }
     void setWakeUpSemaphore(IPC::Semaphore&&);
+    bool hasWakeUpSemaphore() const { return m_wakeUpSemaphore.has_value(); }
 
     void setWakeUpMessageHysteresis(unsigned hysteresis)

trunk/Source/WebKit/WebProcess/GPU/graphics/RemoteRenderingBackendProxy.cpp

@@ -64 +64 @@
     if (!m_gpuProcessConnection)
         return;
-
-    // Un-register itself as a MessageReceiver.
-    m_gpuProcessConnection->messageReceiverMap().removeMessageReceiver(*this);
-
-    // Release the RemoteRenderingBackend.
     m_gpuProcessConnection->connection().send(Messages::GPUConnectionToWebProcess::ReleaseRenderingBackend(renderingBackendIdentifier()), 0, IPC::SendOption::DispatchMessageEvenWhenWaitingForSyncReply);
+    disconnectGPUProcess();
 }
 
@@ -86 +82 @@
 }
 
-void RemoteRenderingBackendProxy::gpuProcessConnectionDidClose(GPUProcessConnection& previousConnection)
-{
-    previousConnection.removeClient(*this);
-    previousConnection.messageReceiverMap().removeMessageReceiver(*this);
+void RemoteRenderingBackendProxy::gpuProcessConnectionDidClose(GPUProcessConnection&)
+{
+    if (!m_gpuProcessConnection)
+        return;
+    disconnectGPUProcess();
+    // Note: The cache will call back to this to setup a new connection.
+    m_remoteResourceCacheProxy.remoteResourceCacheWasDestroyed();
+}
+
+void RemoteRenderingBackendProxy::disconnectGPUProcess()
+{
+    m_gpuProcessConnection->removeClient(*this);
+    m_gpuProcessConnection->messageReceiverMap().removeMessageReceiver(*this);
     m_gpuProcessConnection = nullptr;
-
-    m_remoteResourceCacheProxy.remoteResourceCacheWasDestroyed();
 
     if (m_destroyGetPixelBufferSharedMemoryTimer.isActive())
@@ -99 +102 @@
     m_renderingUpdateID = { };
     m_didRenderingUpdateID = { };
+    m_streamConnection = nullptr;
 }
 
@@ -412 +416 @@
 {
     ensureGPUProcessConnection();
-    if (UNLIKELY(m_needsWakeUpSemaphoreForDisplayListStream))
+    if (UNLIKELY(!m_streamConnection->hasWakeUpSemaphore()))
         m_streamConnection->waitForAndDispatchImmediately<Messages::RemoteRenderingBackendProxy::DidCreateWakeUpSemaphoreForDisplayListStream>(renderingBackendIdentifier(), 3_s, IPC::WaitForOption::InterruptWaitingIfSyncMessageArrives);
     return *m_streamConnection;
@@ -423 +427 @@
         return;
     }
-
     m_streamConnection->setWakeUpSemaphore(WTFMove(semaphore));
-    m_needsWakeUpSemaphoreForDisplayListStream = false;
 }
 

trunk/Source/WebKit/WebProcess/GPU/graphics/RemoteRenderingBackendProxy.h

@@ -160 +160 @@
     // GPUProcessConnection::Client
     void gpuProcessConnectionDidClose(GPUProcessConnection&) final;
-
+    void disconnectGPUProcess();
     GPUProcessConnection& ensureGPUProcessConnection();
 
@@ -182 +182 @@
     RenderingUpdateID m_renderingUpdateID;
     RenderingUpdateID m_didRenderingUpdateID;
-
-    bool m_needsWakeUpSemaphoreForDisplayListStream { true };
 };