Modified: trunk/Source/WebKit/ChangeLog (285232 => 285233)
--- trunk/Source/WebKit/ChangeLog 2021-11-03 22:01:56 UTC (rev 285232)
+++ trunk/Source/WebKit/ChangeLog 2021-11-03 22:03:05 UTC (rev 285233)
@@ -1,3 +1,48 @@
+2021-11-03 Wenson Hsieh <[email protected]>
+
+ [GPU Process] Rendering backend may stop processing IPC stream messages after a sync message timeout
+ https://bugs.webkit.org/show_bug.cgi?id=232580
+
+ Reviewed by Tim Horton.
+
+ Currently, it's possible for sync IPC messages that are sent through IPC streams to time out and subsequently
+ cause the IPC stream server (e.g. RemoteRenderingBackend) to stop processing IPC stream messages altogether. The
+ following example illustrates how this might happen:
+
+ 1. Web process sends the sync message at offset `n` (relative to the start of the ring buffer).
+ Web process then begins to wait for the sync stream response under `tryAcquireAll()`.
+
+ 2. GPU process receives the sync message, and takes a while (> 1 sec.) attempting to dispatch it.
+
+ 3. Meanwhile in the web process, we hit the IPC timeout and bail from `tryAcquireAll()`. The client offset
+ is still at `n`. Shortly afterwards, the web process tries to send a regular stream message, calling
+ `tryAcquire()` and waiting for the client semaphore to be signaled.
+
+ 4. The GPU process finally finishes dispatching the sync message from (2), sends a sync reply (unaware that
+ the web process has already given up waiting), and finally calls `releaseAll()`, signaling the client
+ semaphore. The server offset is then reset back to 0.
+
+ 5. The web process finishes waiting, returns from `tryAcquire()`, proceeds to write the new stream
+ message after offset `n`, and wakes up the GPU process with the intention of handling this message.
+
+ 6. The GPU process wakes up and begins reading from offset 0. The first message it encounters is the
+ `SyncMessageReply` it just wrote in step (4), and subsequently bails from the processing loop because it
+ isn't capable of dispatching this message (which was meant to be consumed in the web process in the
+ first place).
+
+ In summary, the sync IPC timeout causes us to enter a state where the web process continues to encode messages
+ at the buffer offset prior to timeout (i.e. offset `n`), while the GPU process is reading from the start of the
+ buffer (offset `0`). To avoid this, simply replace the timeouts when synchronously grabbing image data from the
+ GPU process. This ensures that we're never in a state where we're continuing to send stream messages to a
+ rendering backend connection stream after timing out when waiting for a previous sync message to that stream to
+ finish.
+
+ * WebProcess/GPU/graphics/RemoteImageBufferProxy.h:
+ * WebProcess/GPU/graphics/RemoteRenderingBackendProxy.cpp:
+ (WebKit::RemoteRenderingBackendProxy::sharedMemoryForGetPixelBuffer):
+ (WebKit::RemoteRenderingBackendProxy::waitForGetPixelBufferToComplete):
+ * WebProcess/GPU/graphics/RemoteRenderingBackendProxy.h:
+
2021-11-03 Tim Horton <[email protected]>
Fix the build.
Modified: trunk/Source/WebKit/WebProcess/GPU/graphics/RemoteImageBufferProxy.h (285232 => 285233)
--- trunk/Source/WebKit/WebProcess/GPU/graphics/RemoteImageBufferProxy.h 2021-11-03 22:01:56 UTC (rev 285232)
+++ trunk/Source/WebKit/WebProcess/GPU/graphics/RemoteImageBufferProxy.h 2021-11-03 22:03:05 UTC (rev 285233)
@@ -208,8 +208,7 @@
return std::nullopt;
size_t dataSize = pixelBuffer->data().byteLength();
- IPC::Timeout timeout = 5_s;
- SharedMemory* sharedMemory = m_remoteRenderingBackendProxy->sharedMemoryForGetPixelBuffer(dataSize, timeout);
+ SharedMemory* sharedMemory = m_remoteRenderingBackendProxy->sharedMemoryForGetPixelBuffer(dataSize);
if (!sharedMemory)
return std::nullopt;
@@ -217,7 +216,7 @@
mutableThis.m_remoteDisplayList.getPixelBuffer(destinationFormat, srcRect);
mutableThis.flushDrawingContextAsync();
- if (m_remoteRenderingBackendProxy->waitForGetPixelBufferToComplete(timeout))
+ if (m_remoteRenderingBackendProxy->waitForGetPixelBufferToComplete())
memcpy(pixelBuffer->data().data(), sharedMemory->data(), dataSize);
else
memset(pixelBuffer->data().data(), 0, dataSize);
Modified: trunk/Source/WebKit/WebProcess/GPU/graphics/RemoteRenderingBackendProxy.cpp (285232 => 285233)
--- trunk/Source/WebKit/WebProcess/GPU/graphics/RemoteRenderingBackendProxy.cpp 2021-11-03 22:01:56 UTC (rev 285232)
+++ trunk/Source/WebKit/WebProcess/GPU/graphics/RemoteRenderingBackendProxy.cpp 2021-11-03 22:03:05 UTC (rev 285233)
@@ -155,7 +155,7 @@
return nullptr;
}
-SharedMemory* RemoteRenderingBackendProxy::sharedMemoryForGetPixelBuffer(size_t dataSize, IPC::Timeout timeout)
+SharedMemory* RemoteRenderingBackendProxy::sharedMemoryForGetPixelBuffer(size_t dataSize)
{
bool needsSharedMemory = !m_getPixelBufferSharedMemory || dataSize > m_getPixelBufferSharedMemoryLength;
bool needsSemaphore = !m_getPixelBufferSemaphore;
@@ -167,11 +167,11 @@
IPC::Semaphore semaphore;
if (needsSharedMemory && needsSemaphore)
- sendSyncToStream(Messages::RemoteRenderingBackend::UpdateSharedMemoryAndSemaphoreForGetPixelBuffer(dataSize), Messages::RemoteRenderingBackend::UpdateSharedMemoryAndSemaphoreForGetPixelBuffer::Reply(handle, semaphore), timeout);
+ sendSyncToStream(Messages::RemoteRenderingBackend::UpdateSharedMemoryAndSemaphoreForGetPixelBuffer(dataSize), Messages::RemoteRenderingBackend::UpdateSharedMemoryAndSemaphoreForGetPixelBuffer::Reply(handle, semaphore));
else if (needsSharedMemory)
- sendSyncToStream(Messages::RemoteRenderingBackend::UpdateSharedMemoryForGetPixelBuffer(dataSize), Messages::RemoteRenderingBackend::UpdateSharedMemoryForGetPixelBuffer::Reply(handle), timeout);
+ sendSyncToStream(Messages::RemoteRenderingBackend::UpdateSharedMemoryForGetPixelBuffer(dataSize), Messages::RemoteRenderingBackend::UpdateSharedMemoryForGetPixelBuffer::Reply(handle));
else if (needsSemaphore)
- sendSyncToStream(Messages::RemoteRenderingBackend::SemaphoreForGetPixelBuffer(), Messages::RemoteRenderingBackend::SemaphoreForGetPixelBuffer::Reply(semaphore), timeout);
+ sendSyncToStream(Messages::RemoteRenderingBackend::SemaphoreForGetPixelBuffer(), Messages::RemoteRenderingBackend::SemaphoreForGetPixelBuffer::Reply(semaphore));
if (!handle.handle.isNull()) {
m_getPixelBufferSharedMemory = SharedMemory::map(handle.handle, SharedMemory::Protection::ReadOnly);
@@ -187,10 +187,10 @@
return m_getPixelBufferSharedMemory.get();
}
-bool RemoteRenderingBackendProxy::waitForGetPixelBufferToComplete(IPC::Timeout timeout)
+bool RemoteRenderingBackendProxy::waitForGetPixelBufferToComplete()
{
ASSERT(m_getPixelBufferSemaphore);
- return m_getPixelBufferSemaphore->waitFor(timeout);
+ return m_getPixelBufferSemaphore->wait();
}
void RemoteRenderingBackendProxy::destroyGetPixelBufferSharedMemory()
@@ -202,7 +202,7 @@
String RemoteRenderingBackendProxy::getDataURLForImageBuffer(const String& mimeType, std::optional<double> quality, PreserveResolution preserveResolution, RenderingResourceIdentifier renderingResourceIdentifier)
{
String urlString;
- sendSyncToStream(Messages::RemoteRenderingBackend::GetDataURLForImageBuffer(mimeType, quality, preserveResolution, renderingResourceIdentifier), Messages::RemoteRenderingBackend::GetDataURLForImageBuffer::Reply(urlString), 1_s);
+ sendSyncToStream(Messages::RemoteRenderingBackend::GetDataURLForImageBuffer(mimeType, quality, preserveResolution, renderingResourceIdentifier), Messages::RemoteRenderingBackend::GetDataURLForImageBuffer::Reply(urlString));
return urlString;
}
@@ -209,7 +209,7 @@
Vector<uint8_t> RemoteRenderingBackendProxy::getDataForImageBuffer(const String& mimeType, std::optional<double> quality, RenderingResourceIdentifier renderingResourceIdentifier)
{
Vector<uint8_t> data;
- sendSyncToStream(Messages::RemoteRenderingBackend::GetDataForImageBuffer(mimeType, quality, renderingResourceIdentifier), Messages::RemoteRenderingBackend::GetDataForImageBuffer::Reply(data), 1_s);
+ sendSyncToStream(Messages::RemoteRenderingBackend::GetDataForImageBuffer(mimeType, quality, renderingResourceIdentifier), Messages::RemoteRenderingBackend::GetDataForImageBuffer::Reply(data));
return data;
}
@@ -216,7 +216,7 @@
RefPtr<ShareableBitmap> RemoteRenderingBackendProxy::getShareableBitmap(RenderingResourceIdentifier imageBuffer, PreserveResolution preserveResolution)
{
ShareableBitmap::Handle handle;
- auto sendResult = sendSyncToStream(Messages::RemoteRenderingBackend::GetShareableBitmapForImageBuffer(imageBuffer, preserveResolution), Messages::RemoteRenderingBackend::GetShareableBitmapForImageBuffer::Reply(handle), 1_s);
+ auto sendResult = sendSyncToStream(Messages::RemoteRenderingBackend::GetShareableBitmapForImageBuffer(imageBuffer, preserveResolution), Messages::RemoteRenderingBackend::GetShareableBitmapForImageBuffer::Reply(handle));
if (handle.isNull())
return { };
ASSERT_UNUSED(sendResult, sendResult);
Modified: trunk/Source/WebKit/WebProcess/GPU/graphics/RemoteRenderingBackendProxy.h (285232 => 285233)
--- trunk/Source/WebKit/WebProcess/GPU/graphics/RemoteRenderingBackendProxy.h 2021-11-03 22:01:56 UTC (rev 285232)
+++ trunk/Source/WebKit/WebProcess/GPU/graphics/RemoteRenderingBackendProxy.h 2021-11-03 22:03:05 UTC (rev 285233)
@@ -74,8 +74,8 @@
RemoteResourceCacheProxy& remoteResourceCacheProxy() { return m_remoteResourceCacheProxy; }
- SharedMemory* sharedMemoryForGetPixelBuffer(size_t dataSize, IPC::Timeout);
- bool waitForGetPixelBufferToComplete(IPC::Timeout);
+ SharedMemory* sharedMemoryForGetPixelBuffer(size_t dataSize);
+ bool waitForGetPixelBufferToComplete();
void destroyGetPixelBufferSharedMemory();
void createRemoteImageBuffer(WebCore::ImageBuffer&);
@@ -140,7 +140,7 @@
IPC::StreamClientConnection& streamConnection();
template<typename T>
- auto sendSyncToStream(T&& message, typename T::Reply&& reply, IPC::Timeout timeout)
+ auto sendSyncToStream(T&& message, typename T::Reply&& reply, IPC::Timeout timeout = { Seconds::infinity() })
{
return streamConnection().sendSync(WTFMove(message), WTFMove(reply), renderingBackendIdentifier(), timeout);
}
