Title: [270775] trunk/Source/WebKit
- Revision
- 270775
- Author
- [email protected]
- Date
- 2020-12-14 10:03:06 -0800 (Mon, 14 Dec 2020)
Log Message
Web process crashes during MotionMark Images when GPU Process for DOM is enabled
https://bugs.webkit.org/show_bug.cgi?id=219838
Reviewed by Simon Fraser.
It's possible to return prematurely with a null backend when waiting for image buffer backend creation under
`RemoteImageBufferProxy::ensureBackendCreated()`. Consider the following scenario (where WEB and GPU denote
events that occur in the web and GPU processes, respectively):
1. (WEB) RemoteImageBufferProxy A is created in the web process.
2. (GPU) RemoteImageBuffer A is created in the GPU process, along with an image buffer backend for A.
3. (WEB) RemoteImageBufferProxy B is created in the web process.
4. (GPU) RemoteImageBuffer B is created in the GPU process, along with an image buffer backend for B.
5. (WEB) Something calls `RemoteImageBufferProxy::ensureBackendCreated()` on B.
6. (WEB) We receive the `RemoteRenderingBackendProxy::DidCreateImageBufferBackend` message for A.
B subsequently finishes waiting for backend creation because it incorrectly believes that its backend has been
initialized, and we end up crashing downstream in `RemoteLayerBackingStore::encode()` because
`ensureBackendCreated()` for B returned null.
To fix this, we adopt a similar strategy to what we use in `waitForDidFlushWithTimeout()`, and repeatedly call
`waitForDidCreateImageBufferBackend()` until the backend has been created (allowing for an arbitrary number of
IPC timeouts or failures). Since this counter is only incremented upon timeout (or any other kind of IPC
communication failure), we'll simply receive `RemoteRenderingBackendProxy::DidCreateImageBufferBackend` messages
until we confirm (in the web process) that the current image buffer's backend has been initialized.
* WebProcess/GPU/graphics/RemoteImageBufferProxy.h:
* WebProcess/GPU/graphics/RemoteRenderingBackendProxy.cpp:
(WebKit::RemoteRenderingBackendProxy::waitForDidCreateImageBufferBackend):
Drive-by fix: clarify this code by returning an enum to indicate whether or not we successfully received a
backend creation IPC response from the GPU process, instead of just returning a bool.
* WebProcess/GPU/graphics/RemoteRenderingBackendProxy.h:
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (270774 => 270775)
--- trunk/Source/WebKit/ChangeLog 2020-12-14 17:49:56 UTC (rev 270774)
+++ trunk/Source/WebKit/ChangeLog 2020-12-14 18:03:06 UTC (rev 270775)
@@ -1,3 +1,40 @@
+2020-12-14 Wenson Hsieh <[email protected]>
+
+ Web process crashes during MotionMark Images when GPU Process for DOM is enabled
+ https://bugs.webkit.org/show_bug.cgi?id=219838
+
+ Reviewed by Simon Fraser.
+
+ It's possible to return prematurely with a null backend when waiting for image buffer backend creation under
+ `RemoteImageBufferProxy::ensureBackendCreated()`. Consider the following scenario (where WEB and GPU denote
+ events that occur in the web and GPU processes, respectively):
+
+ 1. (WEB) RemoteImageBufferProxy A is created in the web process.
+ 2. (GPU) RemoteImageBuffer A is created in the GPU process, along with an image buffer backend for A.
+ 3. (WEB) RemoteImageBufferProxy B is created in the web process.
+ 4. (GPU) RemoteImageBuffer B is created in the GPU process, along with an image buffer backend for B.
+ 5. (WEB) Something calls `RemoteImageBufferProxy::ensureBackendCreated()` on B.
+ 6. (WEB) We receive the `RemoteRenderingBackendProxy::DidCreateImageBufferBackend` message for A.
+
+ B subsequently finishes waiting for backend creation because it incorrectly believes that its backend has been
+ initialized, and we end up crashing downstream in `RemoteLayerBackingStore::encode()` because
+ `ensureBackendCreated()` for B returned null.
+
+ To fix this, we adopt a similar strategy to what we use in `waitForDidFlushWithTimeout()`, and repeatedly call
+ `waitForDidCreateImageBufferBackend()` until the backend has been created (allowing for an arbitrary number of
+ IPC timeouts or failures). Since this counter is only incremented upon timeout (or any other kind of IPC
+ communication failure), we'll simply receive `RemoteRenderingBackendProxy::DidCreateImageBufferBackend` messages
+ until we confirm (in the web process) that the current image buffer's backend has been initialized.
+
+ * WebProcess/GPU/graphics/RemoteImageBufferProxy.h:
+ * WebProcess/GPU/graphics/RemoteRenderingBackendProxy.cpp:
+ (WebKit::RemoteRenderingBackendProxy::waitForDidCreateImageBufferBackend):
+
+ Drive-by fix: clarify this code by returning an enum to indicate whether or not we successfully received a
+ backend creation IPC response from the GPU process, instead of just returning a bool.
+
+ * WebProcess/GPU/graphics/RemoteRenderingBackendProxy.h:
+
2020-12-14 Sihui Liu <[email protected]>
Implement recognizer for SpeechRecognition
Modified: trunk/Source/WebKit/WebProcess/GPU/graphics/RemoteImageBufferProxy.h (270774 => 270775)
--- trunk/Source/WebKit/WebProcess/GPU/graphics/RemoteImageBufferProxy.h 2020-12-14 17:49:56 UTC (rev 270774)
+++ trunk/Source/WebKit/WebProcess/GPU/graphics/RemoteImageBufferProxy.h 2020-12-14 18:03:06 UTC (rev 270775)
@@ -129,8 +129,15 @@
WebCore::ImageBufferBackend* ensureBackendCreated() const override
{
- if (!m_backend && m_remoteRenderingBackendProxy)
- m_remoteRenderingBackendProxy->waitForDidCreateImageBufferBackend();
+ if (!m_remoteRenderingBackendProxy)
+ return m_backend.get();
+
+ static constexpr unsigned maximumTimeoutOrFailureCount = 3;
+ unsigned numberOfTimeoutsOrFailures = 0;
+ while (!m_backend && numberOfTimeoutsOrFailures < maximumTimeoutOrFailureCount) {
+ if (m_remoteRenderingBackendProxy->waitForDidCreateImageBufferBackend() == RemoteRenderingBackendProxy::DidReceiveBackendCreationResult::TimeoutOrIPCFailure)
+ ++numberOfTimeoutsOrFailures;
+ }
return m_backend.get();
}
Modified: trunk/Source/WebKit/WebProcess/GPU/graphics/RemoteRenderingBackendProxy.cpp (270774 => 270775)
--- trunk/Source/WebKit/WebProcess/GPU/graphics/RemoteRenderingBackendProxy.cpp 2020-12-14 17:49:56 UTC (rev 270774)
+++ trunk/Source/WebKit/WebProcess/GPU/graphics/RemoteRenderingBackendProxy.cpp 2020-12-14 18:03:06 UTC (rev 270775)
@@ -105,10 +105,12 @@
return m_renderingBackendIdentifier.toUInt64();
}
-bool RemoteRenderingBackendProxy::waitForDidCreateImageBufferBackend()
+RemoteRenderingBackendProxy::DidReceiveBackendCreationResult RemoteRenderingBackendProxy::waitForDidCreateImageBufferBackend()
{
Ref<IPC::Connection> connection = WebProcess::singleton().ensureGPUProcessConnection().connection();
- return connection->waitForAndDispatchImmediately<Messages::RemoteRenderingBackendProxy::DidCreateImageBufferBackend>(m_renderingBackendIdentifier, 1_s, IPC::WaitForOption::InterruptWaitingIfSyncMessageArrives);
+ if (!connection->waitForAndDispatchImmediately<Messages::RemoteRenderingBackendProxy::DidCreateImageBufferBackend>(m_renderingBackendIdentifier, 1_s, IPC::WaitForOption::InterruptWaitingIfSyncMessageArrives))
+ return DidReceiveBackendCreationResult::TimeoutOrIPCFailure;
+ return DidReceiveBackendCreationResult::ReceivedAnyResponse;
}
bool RemoteRenderingBackendProxy::waitForDidFlush()
Modified: trunk/Source/WebKit/WebProcess/GPU/graphics/RemoteRenderingBackendProxy.h (270774 => 270775)
--- trunk/Source/WebKit/WebProcess/GPU/graphics/RemoteRenderingBackendProxy.h 2020-12-14 17:49:56 UTC (rev 270774)
+++ trunk/Source/WebKit/WebProcess/GPU/graphics/RemoteRenderingBackendProxy.h 2020-12-14 18:03:06 UTC (rev 270775)
@@ -87,7 +87,11 @@
void deleteAllFonts();
void releaseRemoteResource(WebCore::RenderingResourceIdentifier);
- bool waitForDidCreateImageBufferBackend();
+ enum class DidReceiveBackendCreationResult : bool {
+ ReceivedAnyResponse,
+ TimeoutOrIPCFailure
+ };
+ DidReceiveBackendCreationResult waitForDidCreateImageBufferBackend();
bool waitForDidFlush();
private:
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
