Title: [284671] trunk/Source/WebKit
- Revision
- 284671
- Author
- [email protected]
- Date
- 2021-10-22 01:18:15 -0700 (Fri, 22 Oct 2021)
Log Message
StreamConnectionWorkQueue is not thread-safe
https://bugs.webkit.org/show_bug.cgi?id=232068
Patch by Kimmo Kinnunen <[email protected]> on 2021-10-22
Reviewed by Wenson Hsieh.
Make StreamConnectionWorkQueue thread-safe so that
addStreamConnection works from multiple threads.
In the future, this will be called when a new RemoteDisplayListRecorder is
added during the StreamConnectionWorkQueue message processing
invocation for RemoteRenderingBackend::createRemoteImageBuffer.
* Platform/IPC/StreamConnectionWorkQueue.cpp:
(IPC::StreamConnectionWorkQueue::~StreamConnectionWorkQueue):
(IPC::StreamConnectionWorkQueue::dispatch):
(IPC::StreamConnectionWorkQueue::addStreamConnection):
(IPC::StreamConnectionWorkQueue::removeStreamConnection):
Change semantics so that adding and removing connections during
shutdown is ok. This is required for simpler code since
during stop we must dispatch all pending messages, but during
message code we might have unconditional start listening when
the message creates a new RemoteDisplayListRecorder.
As a consequence it's not an error to add a connection,
stop and destroy the work queue without removing it.
(IPC::StreamConnectionWorkQueue::stop):
(IPC::StreamConnectionWorkQueue::startProcessingThread):
(IPC::StreamConnectionWorkQueue::wakeUpProcessingThread): Deleted.
* Platform/IPC/StreamConnectionWorkQueue.h:
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (284670 => 284671)
--- trunk/Source/WebKit/ChangeLog 2021-10-22 08:07:47 UTC (rev 284670)
+++ trunk/Source/WebKit/ChangeLog 2021-10-22 08:18:15 UTC (rev 284671)
@@ -1,3 +1,33 @@
+2021-10-22 Kimmo Kinnunen <[email protected]>
+
+ StreamConnectionWorkQueue is not thread-safe
+ https://bugs.webkit.org/show_bug.cgi?id=232068
+
+ Reviewed by Wenson Hsieh.
+
+ Make StreamConnectionWorkQueue thread-safe so that
+ addStreamConnection works from multiple threads.
+ In the future, this will be called when a new RemoteDisplayListRecorder is
+ added during the StreamConnectionWorkQueue message processing
+ invocation for RemoteRenderingBackend::createRemoteImageBuffer.
+
+ * Platform/IPC/StreamConnectionWorkQueue.cpp:
+ (IPC::StreamConnectionWorkQueue::~StreamConnectionWorkQueue):
+ (IPC::StreamConnectionWorkQueue::dispatch):
+ (IPC::StreamConnectionWorkQueue::addStreamConnection):
+ (IPC::StreamConnectionWorkQueue::removeStreamConnection):
+ Change semantics so that adding and removing connections during
+ shutdown is ok. This is required for simpler code since
+ during stop we must dispatch all pending messages, but during
+ message code we might have unconditional start listening when
+ the message creates a new RemoteDisplayListRecorder.
+ As a consequence it's not an error to add a connection,
+ stop and destroy the work queue without removing it.
+ (IPC::StreamConnectionWorkQueue::stop):
+ (IPC::StreamConnectionWorkQueue::startProcessingThread):
+ (IPC::StreamConnectionWorkQueue::wakeUpProcessingThread): Deleted.
+ * Platform/IPC/StreamConnectionWorkQueue.h:
+
2021-10-21 Alex Christensen <[email protected]>
Add webpushd plist
Modified: trunk/Source/WebKit/Platform/IPC/StreamConnectionWorkQueue.cpp (284670 => 284671)
--- trunk/Source/WebKit/Platform/IPC/StreamConnectionWorkQueue.cpp 2021-10-22 08:07:47 UTC (rev 284670)
+++ trunk/Source/WebKit/Platform/IPC/StreamConnectionWorkQueue.cpp 2021-10-22 08:18:15 UTC (rev 284671)
@@ -33,45 +33,61 @@
{
}
+StreamConnectionWorkQueue::~StreamConnectionWorkQueue()
+{
+ // `StreamConnectionWorkQueue::stop()` should be called if anything has been dispatched or listened to.
+ ASSERT(!m_processingThread);
+}
+
void StreamConnectionWorkQueue::dispatch(WTF::Function<void()>&& function)
{
+ ASSERT(!m_shouldQuit); // Re-entering during shutdown not supported.
{
Locker locker { m_lock };
m_functions.append(WTFMove(function));
- ASSERT(!m_shouldQuit); // Re-entering during shutdown not supported.
+ if (!m_shouldQuit && !m_processingThread) {
+ startProcessingThread();
+ return;
+ }
}
- wakeUpProcessingThread();
+ wakeUp();
+}
-}
void StreamConnectionWorkQueue::addStreamConnection(StreamServerConnectionBase& connection)
{
{
Locker locker { m_lock };
m_connections.add(connection);
- ASSERT(!m_shouldQuit); // Re-entering during shutdown not supported.
+ if (!m_shouldQuit && !m_processingThread) {
+ startProcessingThread();
+ return;
+ }
}
- wakeUpProcessingThread();
+ wakeUp();
}
void StreamConnectionWorkQueue::removeStreamConnection(StreamServerConnectionBase& connection)
{
- ASSERT(m_processingThread);
{
Locker locker { m_lock };
m_connections.remove(connection);
- ASSERT(!m_shouldQuit); // Re-entering during shutdown not supported.
}
- m_wakeUpSemaphore.signal();
+ wakeUp();
}
void StreamConnectionWorkQueue::stop()
{
m_shouldQuit = true;
- if (!m_processingThread)
+ RefPtr<Thread> processingThread;
+ {
+ Locker locker { m_lock };
+ processingThread = WTFMove(m_processingThread);
+ }
+ if (!processingThread)
return;
- m_wakeUpSemaphore.signal();
- m_processingThread->waitForCompletion();
- m_processingThread = nullptr;
+ ASSERT(Thread::current().uid() != processingThread->uid());
+ wakeUp();
+ processingThread->waitForCompletion();
}
void StreamConnectionWorkQueue::wakeUp()
@@ -84,13 +100,8 @@
return m_wakeUpSemaphore;
}
-void StreamConnectionWorkQueue::wakeUpProcessingThread()
+void StreamConnectionWorkQueue::startProcessingThread()
{
- if (m_processingThread) {
- m_wakeUpSemaphore.signal();
- return;
- }
-
auto task = [this]() mutable {
for (;;) {
processStreams();
Modified: trunk/Source/WebKit/Platform/IPC/StreamConnectionWorkQueue.h (284670 => 284671)
--- trunk/Source/WebKit/Platform/IPC/StreamConnectionWorkQueue.h 2021-10-22 08:07:47 UTC (rev 284670)
+++ trunk/Source/WebKit/Platform/IPC/StreamConnectionWorkQueue.h 2021-10-22 08:18:15 UTC (rev 284671)
@@ -44,7 +44,7 @@
}
StreamConnectionWorkQueue(const char*);
- ~StreamConnectionWorkQueue() = default;
+ ~StreamConnectionWorkQueue();
void addStreamConnection(StreamServerConnectionBase&);
void removeStreamConnection(StreamServerConnectionBase&);
@@ -55,17 +55,16 @@
Semaphore& wakeUpSemaphore();
private:
- void wakeUpProcessingThread();
+ void startProcessingThread() WTF_REQUIRES_LOCK(m_lock);
void processStreams();
const char* const m_name;
Semaphore m_wakeUpSemaphore;
- RefPtr<Thread> m_processingThread;
-
std::atomic<bool> m_shouldQuit { false };
Lock m_lock;
+ RefPtr<Thread> m_processingThread WTF_GUARDED_BY_LOCK(m_lock);
Deque<Function<void()>> m_functions WTF_GUARDED_BY_LOCK(m_lock);
HashSet<Ref<StreamServerConnectionBase>> m_connections WTF_GUARDED_BY_LOCK(m_lock);
};
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
