Title: [284681] trunk/Source/WebKit
Revision
284681
Author
commit-qu...@webkit.org
Date
2021-10-22 07:38:42 -0700 (Fri, 22 Oct 2021)

Log Message

StreamConnectionWorkQueue::processStreams() has a incorrect protection ref
https://bugs.webkit.org/show_bug.cgi?id=232070

Patch by Kimmo Kinnunen <kkinnu...@apple.com> on 2021-10-22
Reviewed by Wenson Hsieh.

* GPUProcess/graphics/RemoteRenderingBackend.cpp:
(WebKit::RemoteRenderingBackend::~RemoteRenderingBackend):
Additionally remove redundant protection ref from `RemoteRenderingBackend`
cleanup task. Since `m_workQueue->stop()` is run after dispatching the
task, and since `stop()` waits until queue has executed the
all the tasks, the `m_workQueue` ref outlives the protection ref.

* Platform/IPC/StreamConnectionWorkQueue.cpp:
(IPC::StreamConnectionWorkQueue::processStreams):
Remove the redundant protection ref so it does not cause confusion.
The protection ref cannot hold the last ref, as that would mean
that the `StreamConnectionWorkQueue` thread would run the code to
destroy the work queue itself. There has to be a external ref for
`queue->stop()` that outlives the protection ref, as the `stop()` will
wait until the queue thread stops.

(IPC::StreamConnectionWorkQueue::stopAndWaitForCompletion):
(IPC::StreamConnectionWorkQueue::stop): Deleted.
* Platform/IPC/StreamConnectionWorkQueue.h:
Rename `stop()` to `stopAndWaitForCompletion()` to signify what
the function does.

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (284680 => 284681)


--- trunk/Source/WebKit/ChangeLog	2021-10-22 14:31:05 UTC (rev 284680)
+++ trunk/Source/WebKit/ChangeLog	2021-10-22 14:38:42 UTC (rev 284681)
@@ -1,3 +1,32 @@
+2021-10-22  Kimmo Kinnunen  <kkinnu...@apple.com>
+
+        StreamConnectionWorkQueue::processStreams() has a incorrect protection ref
+        https://bugs.webkit.org/show_bug.cgi?id=232070
+
+        Reviewed by Wenson Hsieh.
+
+        * GPUProcess/graphics/RemoteRenderingBackend.cpp:
+        (WebKit::RemoteRenderingBackend::~RemoteRenderingBackend):
+        Additionally remove redundant protection ref from `RemoteRenderingBackend`
+        cleanup task. Since `m_workQueue->stop()` is run after dispatching the
+        task, and since `stop()` waits until queue has executed the
+        all the tasks, the `m_workQueue` ref outlives the protection ref.
+
+        * Platform/IPC/StreamConnectionWorkQueue.cpp:
+        (IPC::StreamConnectionWorkQueue::processStreams):
+        Remove the redundant protection ref so it does not cause confusion.
+        The protection ref cannot hold the last ref, as that would mean
+        that the `StreamConnectionWorkQueue` thread would run the code to
+        destroy the work queue itself. There has to be a external ref for
+        `queue->stop()` that outlives the protection ref, as the `stop()` will
+        wait until the queue thread stops.
+
+        (IPC::StreamConnectionWorkQueue::stopAndWaitForCompletion):
+        (IPC::StreamConnectionWorkQueue::stop): Deleted.
+        * Platform/IPC/StreamConnectionWorkQueue.h:
+        Rename `stop()` to `stopAndWaitForCompletion()` to signify what
+        the function does.
+
 2021-10-22  Alberto Garcia  <be...@igalia.com>
 
         Unreviewed, fix typo in the WebKitWebInspector documentation.

Modified: trunk/Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.cpp (284680 => 284681)


--- trunk/Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.cpp	2021-10-22 14:31:05 UTC (rev 284680)
+++ trunk/Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.cpp	2021-10-22 14:38:42 UTC (rev 284681)
@@ -98,8 +98,8 @@
 {
     // Make sure we destroy the ResourceCache on the WorkQueue since it gets populated on the WorkQueue.
     // Make sure rendering resource request is released after destroying the cache.
-    m_workQueue->dispatch([workQueue = m_workQueue.copyRef(), renderingResourcesRequest = WTFMove(m_renderingResourcesRequest), remoteResourceCache = WTFMove(m_remoteResourceCache)] { });
-    m_workQueue->stop();
+    m_workQueue->dispatch([renderingResourcesRequest = WTFMove(m_renderingResourcesRequest), remoteResourceCache = WTFMove(m_remoteResourceCache)] { });
+    m_workQueue->stopAndWaitForCompletion();
 }
 
 void RemoteRenderingBackend::stopListeningForIPC()

Modified: trunk/Source/WebKit/Platform/IPC/StreamConnectionWorkQueue.cpp (284680 => 284681)


--- trunk/Source/WebKit/Platform/IPC/StreamConnectionWorkQueue.cpp	2021-10-22 14:31:05 UTC (rev 284680)
+++ trunk/Source/WebKit/Platform/IPC/StreamConnectionWorkQueue.cpp	2021-10-22 14:38:42 UTC (rev 284681)
@@ -35,7 +35,7 @@
 
 StreamConnectionWorkQueue::~StreamConnectionWorkQueue()
 {
-    // `StreamConnectionWorkQueue::stop()` should be called if anything has been dispatched or listened to.
+    // `StreamConnectionWorkQueue::stopAndWaitForCompletion()` should be called if anything has been dispatched or listened to.
     ASSERT(!m_processingThread);
 }
 
@@ -75,7 +75,7 @@
     wakeUp();
 }
 
-void StreamConnectionWorkQueue::stop()
+void StreamConnectionWorkQueue::stopAndWaitForCompletion()
 {
     m_shouldQuit = true;
     RefPtr<Thread> processingThread;
@@ -117,7 +117,6 @@
 
 void StreamConnectionWorkQueue::processStreams()
 {
-    Ref protectedThis = *this;
     constexpr size_t defaultMessageLimit = 1000;
     bool hasMoreToProcess = false;
     do {

Modified: trunk/Source/WebKit/Platform/IPC/StreamConnectionWorkQueue.h (284680 => 284681)


--- trunk/Source/WebKit/Platform/IPC/StreamConnectionWorkQueue.h	2021-10-22 14:31:05 UTC (rev 284680)
+++ trunk/Source/WebKit/Platform/IPC/StreamConnectionWorkQueue.h	2021-10-22 14:38:42 UTC (rev 284681)
@@ -49,7 +49,7 @@
     void removeStreamConnection(StreamServerConnectionBase&);
 
     void dispatch(WTF::Function<void()>&&) final;
-    void stop();
+    void stopAndWaitForCompletion();
 
     void wakeUp();
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to