Changeset 247020 in webkit

Timestamp:

Jul 1, 2019 2:11:29 PM (5 years ago)

Author:

commit-queue@webkit.org

Message:

Add main thread assertions in sendWithAsyncReply code
​https://bugs.webkit.org/show_bug.cgi?id=199324

Patch by Alex Christensen <​achristensen@webkit.org> on 2019-07-01
Reviewed by Sam Weinig.

sendWithAsyncReply can only be used on the main thread because
the CompletionHandler will be called on the main thread, and if it's
called from a background thread, then HashMap corruption will likely happen.
Add assertions to alert developers that they should only call sendWithAsyncReply
from the main thread.

This is responding to good feedback from r237294

• Platform/IPC/Connection.cpp:

(IPC::asyncReplyHandlerMap):
(IPC::nextAsyncReplyHandlerID):
(IPC::addAsyncReplyHandler):
(IPC::clearAsyncReplyHandlers):
(IPC::CompletionHandler<void):

Location:

trunk/Source/WebKit

Files:

• ChangeLog (modified) (1 diff)
• Platform/IPC/Connection.cpp (modified) (5 diffs)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2019-07-01  Alex Christensen  <achristensen@webkit.org>
+
+        Add main thread assertions in sendWithAsyncReply code
+        https://bugs.webkit.org/show_bug.cgi?id=199324
+
+        Reviewed by Sam Weinig.
+
+        sendWithAsyncReply can only be used on the main thread because
+        the CompletionHandler will be called on the main thread, and if it's
+        called from a background thread, then HashMap corruption will likely happen.
+        Add assertions to alert developers that they should only call sendWithAsyncReply
+        from the main thread.
+
+        This is responding to good feedback from r237294
+
+        * Platform/IPC/Connection.cpp:
+        (IPC::asyncReplyHandlerMap):
+        (IPC::nextAsyncReplyHandlerID):
+        (IPC::addAsyncReplyHandler):
+        (IPC::clearAsyncReplyHandlers):
+        (IPC::CompletionHandler<void):
+
 2019-07-01  Eric Carlson  <eric.carlson@apple.com>
 

trunk/Source/WebKit/Platform/IPC/Connection.cpp

@@ -240 +240 @@
 static HashMap<uintptr_t, HashMap<uint64_t, CompletionHandler<void(Decoder*)>>>& asyncReplyHandlerMap()
 {
+    ASSERT(RunLoop::isMain());
     static NeverDestroyed<HashMap<uintptr_t, HashMap<uint64_t, CompletionHandler<void(Decoder*)>>>> map;
     return map.get();
@@ -1128 +1129 @@
 uint64_t nextAsyncReplyHandlerID()
 {
+    ASSERT(RunLoop::isMain());
     static uint64_t identifier { 0 };
     return ++identifier;
@@ -1134 +1136 @@
 void addAsyncReplyHandler(Connection& connection, uint64_t identifier, CompletionHandler<void(Decoder*)>&& completionHandler)
 {
+    ASSERT(RunLoop::isMain());
     auto result = asyncReplyHandlerMap().ensure(reinterpret_cast<uintptr_t>(&connection), [] {
         return HashMap<uint64_t, CompletionHandler<void(Decoder*)>>();
@@ -1142 +1145 @@
 void clearAsyncReplyHandlers(const Connection& connection)
 {
+    ASSERT(RunLoop::isMain());
     auto map = asyncReplyHandlerMap().take(reinterpret_cast<uintptr_t>(&connection));
     for (auto& handler : map.values()) {
@@ -1151 +1155 @@
 CompletionHandler<void(Decoder*)> takeAsyncReplyHandler(Connection& connection, uint64_t identifier)
 {
+    ASSERT(RunLoop::isMain());
     auto iterator = asyncReplyHandlerMap().find(reinterpret_cast<uintptr_t>(&connection));
     if (iterator != asyncReplyHandlerMap().end()) {