[webkit-reviews] review denied: [Bug 204355] Make sendWithAsyncReply() safe to call from any thread : [Attachment 383873] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Nov 19 08:45:33 PST 2019

Alex Christensen <achristensen at apple.com> has denied Chris Dumez
<cdumez at apple.com>'s request for review:
Bug 204355: Make sendWithAsyncReply() safe to call from any thread

Attachment 383873: Patch


--- Comment #2 from Alex Christensen <achristensen at apple.com> ---
Comment on attachment 383873
  --> https://bugs.webkit.org/attachment.cgi?id=383873

View in context: https://bugs.webkit.org/attachment.cgi?id=383873&action=review

> Source/WebKit/Platform/IPC/Connection.cpp:255
> +    ASSERT(asyncReplyHandlerMapLock().isHeld());

JSC often does this by passing an unused LockHolder& as a parameter.

> Source/WebKit/Platform/IPC/Connection.cpp:1145
> +    LockHolder locker(asyncReplyHandlerMapLock());
>      auto map =
>      for (auto& handler : map.values()) {

The lock should only be held during the taking from the map, not during the
iterating and calling.	This needs an additional scope.

> Source/WebKit/Platform/IPC/Connection.h:204
> +    // Main thread only.

Add an assert?

> Source/WebKit/Platform/IPC/Connection.h:211
> +    // Main thread only.


> Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm:320
> +   
sWasResumed(), [this] {

Protect this or capture a weak pointer.  This looks like a UAF waiting to
happen.  Even if there's something else assuring this is safe, it wouldn't

More information about the webkit-reviews mailing list