- Revision
- 276145
- Author
- [email protected]
- Date
- 2021-04-16 11:02:48 -0700 (Fri, 16 Apr 2021)
Log Message
Early IPC messages to a WorkQueueMessageReceiver may get processed out of order
https://bugs.webkit.org/show_bug.cgi?id=224623
Reviewed by Geoffrey Garen.
Bug 224566 exposed an issue where early IPC being sent to WorkQueueMessageReceiver might get received
out of order. The reason behind it is that the WorkQueueMessageReceiver registers itself on the main
thread while we receive the IPC on the IPC thread. When we receive the IPC on the IPC thread, we check
if there is a WorkQueueMessageReceiver for it and if there is, we dispatch the message straight to its
WorkQueue. However, if the WorkQueueMessageReceiver has not registered itself yet on the main thread,
we hop to the main thread first, before dispatching the IPC back to the receiver's WorkQueue. The
extra hop to the main thread means that 2 IPC messages to the WorkQueueMessageReceiver sent one after
the other may get dispatched on the WorkQueue in an inconsistent order, if the WorkQueueMessageReceiver
registers itself as a receiver in between the 2 IPC messages.
We actually were trying to deal with this issue in Connection::addWorkQueueMessageReceiver(). When
the WorkQueueMessageReceiver would register itself on the main thread, we would grab the incomingMessages
lock and check m_incomingMessages for messages that should be dispatched to the WorkQueue. Those are
async messages that should have been dispatched straight to the WorkQueue on the IPC thread but didn't
because the WorkQueueMessageReceiver has not registered itself yet.
However, this logic in Connection::addWorkQueueMessageReceiver() was insufficient because it only checked
m_incomingMessages. m_incomingMessages only contains async messages. Sync messages (and special async
messages with the IPC::SendOption::DispatchMessageEvenWhenWaitingForSyncReply flag) are stored in
Connection::SyncMessageState::m_messagesToDispatchWhileWaitingForSyncReply. This is what was causing
Bug 224566 since RemoteRenderingBackendProxy's CreateImageBuffer IPC was async with the
DispatchMessageEvenWhenWaitingForSyncReply flag and its GetDataURLForImageBuffer was synchronous. The
ordering of these 2 IPC messages could get reversed and it would cause correctness issues and flaky
crashes.
To address the issue, I updated Connection::addWorkQueueMessageReceiver() to ask the
Connection::SyncMessageState to enqueue its matching messages to the WorkQueue. There was one issue
though because Connection::SyncMessageState::dispatchMessagesAndResetDidScheduleDispatchMessagesForConnection()
was taking messages out of m_messagesToDispatchWhileWaitingForSyncReply and storing them in a local
container and then iterating over this container to dispatch the messages. The dispatching of one
of these messages could cause a WorkQueueMessageReceiver to register itself (call addWorkQueueMessageReceiver()).
When this would happen, addWorkQueueMessageReceiver() would try and enqueue matching messages in
m_messagesToDispatchWhileWaitingForSyncReply and would miss the messages in the local container
that dispatchMessagesAndResetDidScheduleDispatchMessagesForConnection() is currently iterating on.
To address this issue, I introduced a new m_messagesBeingDispatched data member and used that to
store the messages being dispatched instead of the local container. As a result,
addWorkQueueMessageReceiver() can now enqueue the messages in m_messagesBeingDispatched first and
then enqueue the ones in m_messagesToDispatchWhileWaitingForSyncReply.
* GPUProcess/GPUConnectionToWebProcess.cpp:
(WebKit::GPUConnectionToWebProcess::createRenderingBackend):
* GPUProcess/GPUConnectionToWebProcess.h:
* GPUProcess/GPUConnectionToWebProcess.messages.in:
* WebProcess/GPU/graphics/RemoteRenderingBackendProxy.cpp:
(WebKit::RemoteRenderingBackendProxy::ensureGPUProcessConnection):
Revert r276007 that was committed as a temporary workaround for this bug.
* Platform/IPC/Connection.cpp:
(IPC::Connection::SyncMessageState::enqueueMatchingMessages):
Add utility function to SyncMessageState to enqueue its matching messages in m_messagesBeingDispatched
and m_messagesToDispatchWhileWaitingForSyncReply to the provided MessageReceiveQueue. This is called
by Connection::addMessageReceiveQueue(). The logic is similar to the one in the
enqueueMatchingMessagesToMessageReceiveQueue() function but works on a Deque<ConnectionAndIncomingMessage>
instead of a Deque<std::unique_ptr<Decoder>>.
(IPC::Connection::SyncMessageState::dispatchMessages):
(IPC::Connection::SyncMessageState::dispatchMessagesAndResetDidScheduleDispatchMessagesForConnection):
Use m_messagesBeingDispatched instead of a local container to store the messages we are about to
dispatch. This allows enqueueMatchingMessages() to check those messages to see if they should be
dispatched to a MessageReceiveQueue. We also need to make sure we don't iterate over
m_messagesBeingDispatched to call dispatch() on the messages. This is important because any message
dispatch may cause a WorkQueueMessageReceiver to register itself, which would call
enqueueMatchingMessages() and potentially extract matching messages from m_messagesBeingDispatched.
For this reason, we take messages from m_messagesBeingDispatched one by one, until the container
becomes empty.
(IPC::Connection::addMessageReceiveQueue):
(IPC::Connection::addWorkQueueMessageReceiver):
(IPC::Connection::addThreadMessageReceiver):
- This used to only check m_incomingMessages for matching messages that should be enqueued on the
MessageReceiveQueue in order to preserve IPC ordering. This was insufficient because it would fail
to consider sync IPC messages (or async IPC messages with the
IPC::SendOption::DispatchMessageEvenWhenWaitingForSyncReply flag). Since those are stored in separate
containers in Connection::SyncMessageState, we now also call
Connection::SyncMessageState::enqueueMatchingMessages() to enqueue those messages and preserve their
ordering too. This fixes IPC ordering bug identified via Bug 224566.
- Avoid some code duplication by moving more logic to a shared enqueueMatchingMessagesToMessageReceiveQueue()
function. The function is no longer templated because I don't think it is worth increasing binary
size just to avoid the virtual enqueueMessage() function call on the MessageReceiverQueue. We do not
register message receivers very often and then only have a few early messages at most to enqueue.
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (276144 => 276145)
--- trunk/Source/WebKit/ChangeLog 2021-04-16 17:53:22 UTC (rev 276144)
+++ trunk/Source/WebKit/ChangeLog 2021-04-16 18:02:48 UTC (rev 276145)
@@ -1,3 +1,91 @@
+2021-04-16 Chris Dumez <[email protected]>
+
+ Early IPC messages to a WorkQueueMessageReceiver may get processed out of order
+ https://bugs.webkit.org/show_bug.cgi?id=224623
+
+ Reviewed by Geoffrey Garen.
+
+ Bug 224566 exposed an issue where early IPC being sent to WorkQueueMessageReceiver might get received
+ out of order. The reason behind it is that the WorkQueueMessageReceiver registers itself on the main
+ thread while we receive the IPC on the IPC thread. When we receive the IPC on the IPC thread, we check
+ if there is a WorkQueueMessageReceiver for it and if there is, we dispatch the message straight to its
+ WorkQueue. However, if the WorkQueueMessageReceiver has not registered itself yet on the main thread,
+ we hop to the main thread first, before dispatching the IPC back to the receiver's WorkQueue. The
+ extra hop to the main thread means that 2 IPC messages to the WorkQueueMessageReceiver sent one after
+ the other may get dispatched on the WorkQueue in an inconsistent order, if the WorkQueueMessageReceiver
+ registers itself as a receiver in between the 2 IPC messages.
+
+ We actually were trying to deal with this issue in Connection::addWorkQueueMessageReceiver(). When
+ the WorkQueueMessageReceiver would register itself on the main thread, we would grab the incomingMessages
+ lock and check m_incomingMessages for messages that should be dispatched to the WorkQueue. Those are
+ async messages that should have been dispatched straight to the WorkQueue on the IPC thread but didn't
+ because the WorkQueueMessageReceiver has not registered itself yet.
+
+ However, this logic in Connection::addWorkQueueMessageReceiver() was insufficient because it only checked
+ m_incomingMessages. m_incomingMessages only contains async messages. Sync messages (and special async
+ messages with the IPC::SendOption::DispatchMessageEvenWhenWaitingForSyncReply flag) are stored in
+ Connection::SyncMessageState::m_messagesToDispatchWhileWaitingForSyncReply. This is what was causing
+ Bug 224566 since RemoteRenderingBackendProxy's CreateImageBuffer IPC was async with the
+ DispatchMessageEvenWhenWaitingForSyncReply flag and its GetDataURLForImageBuffer was synchronous. The
+ ordering of these 2 IPC messages could get reversed and it would cause correctness issues and flaky
+ crashes.
+
+ To address the issue, I updated Connection::addWorkQueueMessageReceiver() to ask the
+ Connection::SyncMessageState to enqueue its matching messages to the WorkQueue. There was one issue
+ though because Connection::SyncMessageState::dispatchMessagesAndResetDidScheduleDispatchMessagesForConnection()
+ was taking messages out of m_messagesToDispatchWhileWaitingForSyncReply and storing them in a local
+ container and then iterating over this container to dispatch the messages. The dispatching of one
+ of these messages could cause a WorkQueueMessageReceiver to register itself (call addWorkQueueMessageReceiver()).
+ When this would happen, addWorkQueueMessageReceiver() would try and enqueue matching messages in
+ m_messagesToDispatchWhileWaitingForSyncReply and would miss the messages in the local container
+ that dispatchMessagesAndResetDidScheduleDispatchMessagesForConnection() is currently iterating on.
+ To address this issue, I introduced a new m_messagesBeingDispatched data member and used that to
+ store the messages being dispatched instead of the local container. As a result,
+ addWorkQueueMessageReceiver() can now enqueue the messages in m_messagesBeingDispatched first and
+ then enqueue the ones in m_messagesToDispatchWhileWaitingForSyncReply.
+
+ * GPUProcess/GPUConnectionToWebProcess.cpp:
+ (WebKit::GPUConnectionToWebProcess::createRenderingBackend):
+ * GPUProcess/GPUConnectionToWebProcess.h:
+ * GPUProcess/GPUConnectionToWebProcess.messages.in:
+ * WebProcess/GPU/graphics/RemoteRenderingBackendProxy.cpp:
+ (WebKit::RemoteRenderingBackendProxy::ensureGPUProcessConnection):
+ Revert r276007 that was committed as a temporary workaround for this bug.
+
+ * Platform/IPC/Connection.cpp:
+ (IPC::Connection::SyncMessageState::enqueueMatchingMessages):
+ Add utility function to SyncMessageState to enqueue its matching messages in m_messagesBeingDispatched
+ and m_messagesToDispatchWhileWaitingForSyncReply to the provided MessageReceiveQueue. This is called
+ by Connection::addMessageReceiveQueue(). The logic is similar to the one in the
+ enqueueMatchingMessagesToMessageReceiveQueue() function but works on a Deque<ConnectionAndIncomingMessage>
+ instead of a Deque<std::unique_ptr<Decoder>>.
+
+ (IPC::Connection::SyncMessageState::dispatchMessages):
+ (IPC::Connection::SyncMessageState::dispatchMessagesAndResetDidScheduleDispatchMessagesForConnection):
+ Use m_messagesBeingDispatched instead of a local container to store the messages we are about to
+ dispatch. This allows enqueueMatchingMessages() to check those messages to see if they should be
+ dispatched to a MessageReceiveQueue. We also need to make sure we don't iterate over
+ m_messagesBeingDispatched to call dispatch() on the messages. This is important because any message
+ dispatch may cause a WorkQueueMessageReceiver to register itself, which would call
+ enqueueMatchingMessages() and potentially extract matching messages from m_messagesBeingDispatched.
+ For this reason, we take messages from m_messagesBeingDispatched one by one, until the container
+ becomes empty.
+
+ (IPC::Connection::addMessageReceiveQueue):
+ (IPC::Connection::addWorkQueueMessageReceiver):
+ (IPC::Connection::addThreadMessageReceiver):
+ - This used to only check m_incomingMessages for matching messages that should be enqueued on the
+ MessageReceiveQueue in order to preserve IPC ordering. This was insufficient because it would fail
+ to consider sync IPC messages (or async IPC messages with the
+ IPC::SendOption::DispatchMessageEvenWhenWaitingForSyncReply flag). Since those are stored in separate
+ containers in Connection::SyncMessageState, we now also call
+ Connection::SyncMessageState::enqueueMatchingMessages() to enqueue those messages and preserve their
+ ordering too. This fixes IPC ordering bug identified via Bug 224566.
+ - Avoid some code duplication by moving more logic to a shared enqueueMatchingMessagesToMessageReceiveQueue()
+ function. The function is no longer templated because I don't think it is worth increasing binary
+ size just to avoid the virtual enqueueMessage() function call on the MessageReceiverQueue. We do not
+ register message receivers very often and then only have a few early messages at most to enqueue.
+
2021-04-16 Carlos Garcia Campos <[email protected]>
[SOUP] Show resource priority and remote IP in the inspector
Modified: trunk/Source/WebKit/GPUProcess/GPUConnectionToWebProcess.cpp (276144 => 276145)
--- trunk/Source/WebKit/GPUProcess/GPUConnectionToWebProcess.cpp 2021-04-16 17:53:22 UTC (rev 276144)
+++ trunk/Source/WebKit/GPUProcess/GPUConnectionToWebProcess.cpp 2021-04-16 18:02:48 UTC (rev 276145)
@@ -362,13 +362,12 @@
}
#endif
-void GPUConnectionToWebProcess::createRenderingBackend(RemoteRenderingBackendCreationParameters&& creationParameters, CompletionHandler<void()>&& completionHandler)
+void GPUConnectionToWebProcess::createRenderingBackend(RemoteRenderingBackendCreationParameters&& creationParameters)
{
auto addResult = m_remoteRenderingBackendMap.ensure(creationParameters.identifier, [&]() {
return IPC::ScopedActiveMessageReceiveQueue { RemoteRenderingBackend::create(*this, WTFMove(creationParameters)) };
});
ASSERT_UNUSED(addResult, addResult.isNewEntry);
- completionHandler();
}
void GPUConnectionToWebProcess::releaseRenderingBackend(RenderingBackendIdentifier renderingBackendIdentifier)
Modified: trunk/Source/WebKit/GPUProcess/GPUConnectionToWebProcess.h (276144 => 276145)
--- trunk/Source/WebKit/GPUProcess/GPUConnectionToWebProcess.h 2021-04-16 17:53:22 UTC (rev 276144)
+++ trunk/Source/WebKit/GPUProcess/GPUConnectionToWebProcess.h 2021-04-16 18:02:48 UTC (rev 276145)
@@ -161,7 +161,7 @@
#endif
#endif
- void createRenderingBackend(RemoteRenderingBackendCreationParameters&&, CompletionHandler<void()>&&);
+ void createRenderingBackend(RemoteRenderingBackendCreationParameters&&);
void releaseRenderingBackend(RenderingBackendIdentifier);
#if ENABLE(WEBGL)
Modified: trunk/Source/WebKit/GPUProcess/GPUConnectionToWebProcess.messages.in (276144 => 276145)
--- trunk/Source/WebKit/GPUProcess/GPUConnectionToWebProcess.messages.in 2021-04-16 17:53:22 UTC (rev 276144)
+++ trunk/Source/WebKit/GPUProcess/GPUConnectionToWebProcess.messages.in 2021-04-16 18:02:48 UTC (rev 276145)
@@ -23,7 +23,7 @@
#if ENABLE(GPU_PROCESS)
messages -> GPUConnectionToWebProcess WantsDispatchMessage {
- void CreateRenderingBackend(struct WebKit::RemoteRenderingBackendCreationParameters creationParameters) -> () Synchronous
+ void CreateRenderingBackend(struct WebKit::RemoteRenderingBackendCreationParameters creationParameters)
void ReleaseRenderingBackend(WebKit::RenderingBackendIdentifier renderingBackendIdentifier)
#if ENABLE(WEBGL)
void CreateGraphicsContextGL(struct WebCore::GraphicsContextGLAttributes attributes, WebKit::GraphicsContextGLIdentifier graphicsContextGLIdentifier, WebKit::RenderingBackendIdentifier renderingBackendIdentifier, IPC::StreamConnectionBuffer stream)
Modified: trunk/Source/WebKit/Platform/IPC/Connection.cpp (276144 => 276145)
--- trunk/Source/WebKit/Platform/IPC/Connection.cpp 2021-04-16 17:53:22 UTC (rev 276144)
+++ trunk/Source/WebKit/Platform/IPC/Connection.cpp 2021-04-16 18:02:48 UTC (rev 276145)
@@ -93,6 +93,9 @@
// Dispatch pending sync messages.
void dispatchMessages();
+ // Add matching pending messages to the provided MessageReceiveQueue.
+ void enqueueMatchingMessages(Connection&, MessageReceiveQueue&, ReceiverName, uint64_t destinationID);
+
private:
friend class LazyNeverDestroyed<Connection::SyncMessageState>;
SyncMessageState() = default;
@@ -117,7 +120,8 @@
connection->dispatchMessage(WTFMove(message));
}
};
- Vector<ConnectionAndIncomingMessage> m_messagesToDispatchWhileWaitingForSyncReply;
+ Deque<ConnectionAndIncomingMessage> m_messagesBeingDispatched; // Only used on the main thread.
+ Deque<ConnectionAndIncomingMessage> m_messagesToDispatchWhileWaitingForSyncReply;
};
Connection::SyncMessageState& Connection::SyncMessageState::singleton()
@@ -132,6 +136,24 @@
return syncMessageState;
}
+void Connection::SyncMessageState::enqueueMatchingMessages(Connection& connection, MessageReceiveQueue& receiveQueue, ReceiverName receiverName, uint64_t destinationID)
+{
+ ASSERT(isMainRunLoop());
+ auto enqueueMatchingMessagesInContainer = [&](Deque<ConnectionAndIncomingMessage>& connectionAndMessages) {
+ Deque<ConnectionAndIncomingMessage> rest;
+ for (auto& connectionAndMessage : connectionAndMessages) {
+ if (connectionAndMessage.connection.ptr() == &connection && connectionAndMessage.message->messageReceiverName() == receiverName && (connectionAndMessage.message->destinationID() == destinationID || !destinationID))
+ receiveQueue.enqueueMessage(connection, WTFMove(connectionAndMessage.message));
+ else
+ rest.append(WTFMove(connectionAndMessage));
+ }
+ connectionAndMessages = WTFMove(rest);
+ };
+ auto locker = holdLock(m_mutex);
+ enqueueMatchingMessagesInContainer(m_messagesBeingDispatched);
+ enqueueMatchingMessagesInContainer(m_messagesToDispatchWhileWaitingForSyncReply);
+}
+
bool Connection::SyncMessageState::processIncomingMessage(Connection& connection, std::unique_ptr<Decoder>& message)
{
switch (message->shouldDispatchMessageWhenWaitingForSyncReply()) {
@@ -173,14 +195,18 @@
{
ASSERT(RunLoop::isMain());
- Vector<ConnectionAndIncomingMessage> messagesToDispatchWhileWaitingForSyncReply;
{
auto locker = holdLock(m_mutex);
- m_messagesToDispatchWhileWaitingForSyncReply.swap(messagesToDispatchWhileWaitingForSyncReply);
+ if (m_messagesBeingDispatched.isEmpty())
+ m_messagesBeingDispatched = std::exchange(m_messagesToDispatchWhileWaitingForSyncReply, { });
+ else {
+ while (!m_messagesToDispatchWhileWaitingForSyncReply.isEmpty())
+ m_messagesBeingDispatched.append(m_messagesToDispatchWhileWaitingForSyncReply.takeLast());
+ }
}
- for (auto& connectionAndIncomingMessage : messagesToDispatchWhileWaitingForSyncReply)
- connectionAndIncomingMessage.dispatch();
+ while (!m_messagesBeingDispatched.isEmpty())
+ m_messagesBeingDispatched.takeFirst().dispatch();
}
void Connection::SyncMessageState::dispatchMessagesAndResetDidScheduleDispatchMessagesForConnection(Connection& connection)
@@ -187,27 +213,23 @@
{
ASSERT(RunLoop::isMain());
- Vector<ConnectionAndIncomingMessage> messagesToDispatchWhileWaitingForSyncReply;
{
auto locker = holdLock(m_mutex);
ASSERT(m_didScheduleDispatchMessagesWorkSet.contains(&connection));
m_didScheduleDispatchMessagesWorkSet.remove(&connection);
- m_messagesToDispatchWhileWaitingForSyncReply.swap(messagesToDispatchWhileWaitingForSyncReply);
+ ASSERT(m_messagesBeingDispatched.isEmpty());
+ Deque<ConnectionAndIncomingMessage> messagesToPutBack;
+ for (auto& connectionAndIncomingMessage : m_messagesToDispatchWhileWaitingForSyncReply) {
+ if (&connection == connectionAndIncomingMessage.connection.ptr())
+ m_messagesBeingDispatched.append(WTFMove(connectionAndIncomingMessage));
+ else
+ messagesToPutBack.append(WTFMove(connectionAndIncomingMessage));
+ }
+ m_messagesToDispatchWhileWaitingForSyncReply = WTFMove(messagesToPutBack);
}
- Vector<ConnectionAndIncomingMessage> messagesToPutBack;
- for (auto& connectionAndIncomingMessage : messagesToDispatchWhileWaitingForSyncReply) {
- if (&connection == connectionAndIncomingMessage.connection.ptr())
- connectionAndIncomingMessage.dispatch();
- else
- messagesToPutBack.append(WTFMove(connectionAndIncomingMessage));
- }
-
- if (!messagesToPutBack.isEmpty()) {
- auto locker = holdLock(m_mutex);
- messagesToPutBack.appendVector(WTFMove(m_messagesToDispatchWhileWaitingForSyncReply));
- m_messagesToDispatchWhileWaitingForSyncReply = WTFMove(messagesToPutBack);
- }
+ while (!m_messagesBeingDispatched.isEmpty())
+ m_messagesBeingDispatched.takeFirst().dispatch();
}
// Represents a sync request for which we're waiting on a reply.
@@ -319,25 +341,28 @@
m_shouldExitOnSyncMessageSendFailure = shouldExitOnSyncMessageSendFailure;
}
-namespace {
-template <typename T>
-Deque<std::unique_ptr<Decoder>> filterWithMessageReceiveQueue(Connection& connection, T& receiveQueue, ReceiverName receiverName, uint64_t destinationID, Deque<std::unique_ptr<Decoder>>&& incomingMessages)
+// Enqueue any pending message to the MessageReceiveQueue that is meant to go on that queue. This is important to maintain the ordering of
+// IPC messages as some messages may get received on the IPC thread before the message receiver registered itself on the main thread.
+void Connection::enqueueMatchingMessagesToMessageReceiveQueue(Locker<Lock>&, MessageReceiveQueue& receiveQueue, ReceiverName receiverName, uint64_t destinationID)
{
- Deque<std::unique_ptr<Decoder>> rest;
- for (auto& message : incomingMessages) {
+ ASSERT(isMainRunLoop());
+
+ SyncMessageState::singleton().enqueueMatchingMessages(*this, receiveQueue, receiverName, destinationID);
+
+ Deque<std::unique_ptr<Decoder>> remainingIncomingMessages;
+ for (auto& message : m_incomingMessages) {
if (message->messageReceiverName() == receiverName && (message->destinationID() == destinationID || !destinationID))
- receiveQueue.enqueueMessage(connection, WTFMove(message));
+ receiveQueue.enqueueMessage(*this, WTFMove(message));
else
- rest.append(WTFMove(message));
+ remainingIncomingMessages.append(WTFMove(message));
}
- return rest;
+ m_incomingMessages = WTFMove(remainingIncomingMessages);
}
-}
void Connection::addMessageReceiveQueue(MessageReceiveQueue& receiveQueue, ReceiverName receiverName, uint64_t destinationID)
{
- auto locker = holdLock(m_incomingMessagesMutex);
- m_incomingMessages = filterWithMessageReceiveQueue(*this, receiveQueue, receiverName, destinationID, WTFMove(m_incomingMessages));
+ auto incomingMessagesLocker = holdLock(m_incomingMessagesMutex);
+ enqueueMatchingMessagesToMessageReceiveQueue(incomingMessagesLocker, receiveQueue, receiverName, destinationID);
m_receiveQueues.add(receiveQueue, receiverName, destinationID);
}
@@ -344,8 +369,8 @@
void Connection::addWorkQueueMessageReceiver(ReceiverName receiverName, WorkQueue& workQueue, WorkQueueMessageReceiver* receiver, uint64_t destinationID)
{
auto receiveQueue = makeUnique<WorkQueueMessageReceiverQueue>(workQueue, *receiver);
- auto locker = holdLock(m_incomingMessagesMutex);
- m_incomingMessages = filterWithMessageReceiveQueue(*this, *receiveQueue, receiverName, destinationID, WTFMove(m_incomingMessages));
+ auto incomingMessagesLocker = holdLock(m_incomingMessagesMutex);
+ enqueueMatchingMessagesToMessageReceiveQueue(incomingMessagesLocker, *receiveQueue, receiverName, destinationID);
m_receiveQueues.add(WTFMove(receiveQueue), receiverName, destinationID);
}
@@ -352,8 +377,8 @@
void Connection::addThreadMessageReceiver(ReceiverName receiverName, ThreadMessageReceiver* receiver, uint64_t destinationID)
{
auto receiveQueue = makeUnique<ThreadMessageReceiverQueue>(*receiver);
- auto locker = holdLock(m_incomingMessagesMutex);
- m_incomingMessages = filterWithMessageReceiveQueue(*this, *receiveQueue, receiverName, destinationID, WTFMove(m_incomingMessages));
+ auto incomingMessagesLocker = holdLock(m_incomingMessagesMutex);
+ enqueueMatchingMessagesToMessageReceiveQueue(incomingMessagesLocker, *receiveQueue, receiverName, destinationID);
m_receiveQueues.add(WTFMove(receiveQueue), receiverName, destinationID);
}
Modified: trunk/Source/WebKit/Platform/IPC/Connection.h (276144 => 276145)
--- trunk/Source/WebKit/Platform/IPC/Connection.h 2021-04-16 17:53:22 UTC (rev 276144)
+++ trunk/Source/WebKit/Platform/IPC/Connection.h 2021-04-16 18:02:48 UTC (rev 276145)
@@ -329,6 +329,8 @@
void popPendingSyncRequestID(uint64_t syncRequestID);
std::unique_ptr<Decoder> waitForSyncReply(uint64_t syncRequestID, MessageName, Timeout, OptionSet<SendSyncOption>);
+ void enqueueMatchingMessagesToMessageReceiveQueue(Locker<Lock>& incomingMessagesLocker, MessageReceiveQueue&, ReceiverName, uint64_t destinationID);
+
// Called on the connection work queue.
void processIncomingMessage(std::unique_ptr<Decoder>);
void processIncomingSyncReply(std::unique_ptr<Decoder>);
Modified: trunk/Source/WebKit/WebProcess/GPU/graphics/RemoteRenderingBackendProxy.cpp (276144 => 276145)
--- trunk/Source/WebKit/WebProcess/GPU/graphics/RemoteRenderingBackendProxy.cpp 2021-04-16 17:53:22 UTC (rev 276144)
+++ trunk/Source/WebKit/WebProcess/GPU/graphics/RemoteRenderingBackendProxy.cpp 2021-04-16 18:02:48 UTC (rev 276145)
@@ -75,10 +75,7 @@
auto& gpuProcessConnection = WebProcess::singleton().ensureGPUProcessConnection();
gpuProcessConnection.addClient(*this);
gpuProcessConnection.messageReceiverMap().addMessageReceiver(Messages::RemoteRenderingBackendProxy::messageReceiverName(), renderingBackendIdentifier().toUInt64(), *this);
- // This message is synchronous to ensure that the RemoteRenderingBackend has been created and has registered itself as a WorkQueueMessageReceiver before we send it IPC.
- // Without this synchronization, some IPC messages may get received by the GPUProcess before the RemoteRenderingBackend has registered itself as a WorkQueueMessageReceiver
- // and IPC may get processed out of order.
- gpuProcessConnection.connection().sendSync(Messages::GPUConnectionToWebProcess::CreateRenderingBackend(m_parameters), Messages::GPUConnectionToWebProcess::CreateRenderingBackend::Reply(), 0);
+ gpuProcessConnection.connection().send(Messages::GPUConnectionToWebProcess::CreateRenderingBackend(m_parameters), 0, IPC::SendOption::DispatchMessageEvenWhenWaitingForSyncReply);
m_gpuProcessConnection = makeWeakPtr(gpuProcessConnection);
}
return *m_gpuProcessConnection;
