Diff
Modified: trunk/Source/WebKit/ChangeLog (252635 => 252636)
--- trunk/Source/WebKit/ChangeLog 2019-11-19 18:10:34 UTC (rev 252635)
+++ trunk/Source/WebKit/ChangeLog 2019-11-19 18:23:18 UTC (rev 252636)
@@ -1,3 +1,38 @@
+2019-11-19 Chris Dumez <[email protected]>
+
+ Make sendWithAsyncReply() safe to call from any thread
+ https://bugs.webkit.org/show_bug.cgi?id=204355
+
+ Reviewed by Alex Christensen.
+
+ Make sendWithAsyncReply() safe to call from any thread, similarly to the regular send().
+ Also start using it in WebProcess::processTaskStateDidChange() now that it is safe.
+
+ * Platform/IPC/Connection.cpp:
+ (IPC::asyncReplyHandlerMapLock):
+ (IPC::asyncReplyHandlerMap):
+ (IPC::nextAsyncReplyHandlerID):
+ (IPC::addAsyncReplyHandler):
+ (IPC::clearAsyncReplyHandlers):
+ (IPC::CompletionHandler<void):
+ * UIProcess/Cocoa/WebProcessProxyCocoa.mm:
+ (WebKit::WebProcessProxy::processWasResumed):
+ * UIProcess/WebProcessProxy.h:
+ (WebKit::WebProcessProxy::hasServiceWorkerPageProxy): Deleted.
+ (WebKit::WebProcessProxy::setAssertionStateForTesting): Deleted.
+ (WebKit::WebProcessProxy::WeakOrStrongPtr::WeakOrStrongPtr): Deleted.
+ (WebKit::WebProcessProxy::WeakOrStrongPtr::setIsWeak): Deleted.
+ (WebKit::WebProcessProxy::WeakOrStrongPtr::get const): Deleted.
+ (WebKit::WebProcessProxy::WeakOrStrongPtr::operator-> const): Deleted.
+ (WebKit::WebProcessProxy::WeakOrStrongPtr::operator* const): Deleted.
+ (WebKit::WebProcessProxy::WeakOrStrongPtr::operator bool const): Deleted.
+ (WebKit::WebProcessProxy::WeakOrStrongPtr::updateStrongReference): Deleted.
+ * WebProcess/WebProcess.h:
+ * WebProcess/WebProcess.messages.in:
+ * WebProcess/cocoa/WebProcessCocoa.mm:
+ (WebKit::WebProcess::processTaskStateDidChange):
+ (WebKit::WebProcess::parentProcessDidHandleProcessWasResumed): Deleted.
+
2019-11-19 Megan Gardner <[email protected]>
<rdar://problem/57323799>
Modified: trunk/Source/WebKit/Platform/IPC/Connection.cpp (252635 => 252636)
--- trunk/Source/WebKit/Platform/IPC/Connection.cpp 2019-11-19 18:10:34 UTC (rev 252635)
+++ trunk/Source/WebKit/Platform/IPC/Connection.cpp 2019-11-19 18:23:18 UTC (rev 252636)
@@ -30,6 +30,7 @@
#include "MessageFlags.h"
#include <memory>
#include <wtf/HashSet.h>
+#include <wtf/Lock.h>
#include <wtf/NeverDestroyed.h>
#include <wtf/RunLoop.h>
#include <wtf/text/WTFString.h>
@@ -243,9 +244,15 @@
return map;
}
-static HashMap<uintptr_t, HashMap<uint64_t, CompletionHandler<void(Decoder*)>>>& asyncReplyHandlerMap()
+static Lock& asyncReplyHandlerMapLock()
{
- ASSERT(RunLoop::isMain());
+ static Lock lock;
+ return lock;
+}
+
+static HashMap<uintptr_t, HashMap<uint64_t, CompletionHandler<void(Decoder*)>>>& asyncReplyHandlerMap(const LockHolder&)
+{
+ ASSERT(asyncReplyHandlerMapLock().isHeld());
static NeverDestroyed<HashMap<uintptr_t, HashMap<uint64_t, CompletionHandler<void(Decoder*)>>>> map;
return map.get();
}
@@ -1118,15 +1125,14 @@
uint64_t nextAsyncReplyHandlerID()
{
- ASSERT(RunLoop::isMain());
- static uint64_t identifier { 0 };
+ static std::atomic<uint64_t> identifier { 0 };
return ++identifier;
}
void addAsyncReplyHandler(Connection& connection, uint64_t identifier, CompletionHandler<void(Decoder*)>&& completionHandler)
{
- RELEASE_ASSERT(RunLoop::isMain());
- auto result = asyncReplyHandlerMap().ensure(reinterpret_cast<uintptr_t>(&connection), [] {
+ LockHolder locker(asyncReplyHandlerMapLock());
+ auto result = asyncReplyHandlerMap(locker).ensure(reinterpret_cast<uintptr_t>(&connection), [] {
return HashMap<uint64_t, CompletionHandler<void(Decoder*)>>();
}).iterator->value.add(identifier, WTFMove(completionHandler));
ASSERT_UNUSED(result, result.isNewEntry);
@@ -1134,8 +1140,12 @@
void clearAsyncReplyHandlers(const Connection& connection)
{
- RELEASE_ASSERT(RunLoop::isMain());
- auto map = asyncReplyHandlerMap().take(reinterpret_cast<uintptr_t>(&connection));
+ HashMap<uint64_t, CompletionHandler<void(Decoder*)>> map;
+ {
+ LockHolder locker(asyncReplyHandlerMapLock());
+ map = asyncReplyHandlerMap(locker).take(reinterpret_cast<uintptr_t>(&connection));
+ }
+
for (auto& handler : map.values()) {
if (handler)
handler(nullptr);
@@ -1144,9 +1154,10 @@
CompletionHandler<void(Decoder*)> takeAsyncReplyHandler(Connection& connection, uint64_t identifier)
{
- RELEASE_ASSERT(RunLoop::isMain());
- auto iterator = asyncReplyHandlerMap().find(reinterpret_cast<uintptr_t>(&connection));
- if (iterator != asyncReplyHandlerMap().end()) {
+ LockHolder locker(asyncReplyHandlerMapLock());
+ auto& map = asyncReplyHandlerMap(locker);
+ auto iterator = map.find(reinterpret_cast<uintptr_t>(&connection));
+ if (iterator != map.end()) {
if (!iterator->value.isValidKey(identifier)) {
ASSERT_NOT_REACHED();
connection.markCurrentlyDispatchedMessageAsInvalid();
Modified: trunk/Source/WebKit/Platform/IPC/Connection.h (252635 => 252636)
--- trunk/Source/WebKit/Platform/IPC/Connection.h 2019-11-19 18:10:34 UTC (rev 252635)
+++ trunk/Source/WebKit/Platform/IPC/Connection.h 2019-11-19 18:23:18 UTC (rev 252636)
@@ -182,11 +182,12 @@
void postConnectionDidCloseOnConnectionWorkQueue();
- template<typename T, typename C> void sendWithAsyncReply(T&& message, C&& completionHandler, uint64_t destinationID = 0, OptionSet<SendOption> = { });
- template<typename T> bool send(T&& message, uint64_t destinationID, OptionSet<SendOption> sendOptions = { });
- template<typename T> bool sendSync(T&& message, typename T::Reply&& reply, uint64_t destinationID, Seconds timeout = Seconds::infinity(), OptionSet<SendSyncOption> sendSyncOptions = { });
- template<typename T> bool waitForAndDispatchImmediately(uint64_t destinationID, Seconds timeout, OptionSet<WaitForOption> waitForOptions = { });
+ template<typename T, typename C> void sendWithAsyncReply(T&& message, C&& completionHandler, uint64_t destinationID = 0, OptionSet<SendOption> = { }); // Thread-safe.
+ template<typename T> bool send(T&& message, uint64_t destinationID, OptionSet<SendOption> sendOptions = { }); // Thread-safe.
+ template<typename T> bool sendSync(T&& message, typename T::Reply&& reply, uint64_t destinationID, Seconds timeout = Seconds::infinity(), OptionSet<SendSyncOption> sendSyncOptions = { }); // Main thread only.
+ template<typename T> bool waitForAndDispatchImmediately(uint64_t destinationID, Seconds timeout, OptionSet<WaitForOption> waitForOptions = { }); // Main thread only.
+ // Thread-safe.
template<typename T, typename C, typename U>
void sendWithAsyncReply(T&& message, C&& completionHandler, ObjectIdentifier<U> destinationID = { }, OptionSet<SendOption> sendOptions = { })
{
@@ -193,6 +194,7 @@
sendWithAsyncReply<T, C>(WTFMove(message), WTFMove(completionHandler), destinationID.toUInt64(), sendOptions);
}
+ // Thread-safe.
template<typename T, typename U>
bool send(T&& message, ObjectIdentifier<U> destinationID, OptionSet<SendOption> sendOptions = { })
{
@@ -199,6 +201,7 @@
return send<T>(WTFMove(message), destinationID.toUInt64(), sendOptions);
}
+ // Main thread only.
template<typename T, typename U>
bool sendSync(T&& message, typename T::Reply&& reply, ObjectIdentifier<U> destinationID, Seconds timeout = Seconds::infinity(), OptionSet<SendSyncOption> sendSyncOptions = { })
{
@@ -205,6 +208,7 @@
return sendSync<T>(WTFMove(message), WTFMove(reply), destinationID.toUInt64(), timeout, sendSyncOptions);
}
+ // Main thread only.
template<typename T, typename U>
bool waitForAndDispatchImmediately(ObjectIdentifier<U> destinationID, Seconds timeout, OptionSet<WaitForOption> waitForOptions = { })
{
@@ -483,6 +487,7 @@
template<typename T> bool Connection::sendSync(T&& message, typename T::Reply&& reply, uint64_t destinationID, Seconds timeout, OptionSet<SendSyncOption> sendSyncOptions)
{
COMPILE_ASSERT(T::isSync, SyncMessageExpected);
+ RELEASE_ASSERT(RunLoop::isMain());
uint64_t syncRequestID = 0;
std::unique_ptr<Encoder> encoder = createSyncMessageEncoder(T::receiverName(), T::name(), destinationID, syncRequestID);
@@ -511,6 +516,7 @@
template<typename T> bool Connection::waitForAndDispatchImmediately(uint64_t destinationID, Seconds timeout, OptionSet<WaitForOption> waitForOptions)
{
+ RELEASE_ASSERT(RunLoop::isMain());
std::unique_ptr<Decoder> decoder = waitForMessage(T::receiverName(), T::name(), destinationID, timeout, waitForOptions);
if (!decoder)
return false;
Modified: trunk/Source/WebKit/UIProcess/Cocoa/WebProcessProxyCocoa.mm (252635 => 252636)
--- trunk/Source/WebKit/UIProcess/Cocoa/WebProcessProxyCocoa.mm 2019-11-19 18:10:34 UTC (rev 252635)
+++ trunk/Source/WebKit/UIProcess/Cocoa/WebProcessProxyCocoa.mm 2019-11-19 18:23:18 UTC (rev 252636)
@@ -191,11 +191,9 @@
#endif
#if PLATFORM(IOS_FAMILY)
-void WebProcessProxy::processWasResumed()
+void WebProcessProxy::processWasResumed(CompletionHandler<void()>&& completionHandler)
{
- auto exitScope = makeScopeExit([this] {
- send(Messages::WebProcess::ParentProcessDidHandleProcessWasResumed(), 0);
- });
+ CompletionHandlerCallingScope exitScope(WTFMove(completionHandler));
if (m_throttler.shouldBeRunnable()) {
// The process becoming unsuspended was not unexpected.
Modified: trunk/Source/WebKit/UIProcess/WebProcessProxy.h (252635 => 252636)
--- trunk/Source/WebKit/UIProcess/WebProcessProxy.h 2019-11-19 18:10:34 UTC (rev 252635)
+++ trunk/Source/WebKit/UIProcess/WebProcessProxy.h 2019-11-19 18:23:18 UTC (rev 252636)
@@ -326,7 +326,7 @@
#endif
#if PLATFORM(IOS_FAMILY)
- void processWasResumed();
+ void processWasResumed(CompletionHandler<void()>&&);
#endif
void webPageMediaStateDidChange(WebPageProxy&);
Modified: trunk/Source/WebKit/UIProcess/WebProcessProxy.messages.in (252635 => 252636)
--- trunk/Source/WebKit/UIProcess/WebProcessProxy.messages.in 2019-11-19 18:10:34 UTC (rev 252635)
+++ trunk/Source/WebKit/UIProcess/WebProcessProxy.messages.in 2019-11-19 18:23:18 UTC (rev 252636)
@@ -69,7 +69,7 @@
#endif
#if PLATFORM(IOS_FAMILY)
- ProcessWasResumed()
+ ProcessWasResumed() -> () Async
#endif
# Plug-in messages.
Modified: trunk/Source/WebKit/WebProcess/WebProcess.h (252635 => 252636)
--- trunk/Source/WebKit/WebProcess/WebProcess.h 2019-11-19 18:10:34 UTC (rev 252635)
+++ trunk/Source/WebKit/WebProcess/WebProcess.h 2019-11-19 18:23:18 UTC (rev 252636)
@@ -462,7 +462,6 @@
#if PLATFORM(IOS_FAMILY)
void processTaskStateDidChange(ProcessTaskStateObserver::TaskState) final;
- void parentProcessDidHandleProcessWasResumed();
bool shouldFreezeOnSuspension() const;
void updateFreezerStatus();
#endif
Modified: trunk/Source/WebKit/WebProcess/WebProcess.messages.in (252635 => 252636)
--- trunk/Source/WebKit/WebProcess/WebProcess.messages.in 2019-11-19 18:10:34 UTC (rev 252635)
+++ trunk/Source/WebKit/WebProcess/WebProcess.messages.in 2019-11-19 18:23:18 UTC (rev 252636)
@@ -154,7 +154,6 @@
#if PLATFORM(IOS_FAMILY)
UnblockAccessibilityServer(WebKit::SandboxExtension::Handle handle)
- ParentProcessDidHandleProcessWasResumed();
#endif
#if PLATFORM(GTK) || PLATFORM(WPE)
Modified: trunk/Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm (252635 => 252636)
--- trunk/Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm 2019-11-19 18:10:34 UTC (rev 252635)
+++ trunk/Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm 2019-11-19 18:23:18 UTC (rev 252636)
@@ -317,21 +317,15 @@
};
// This will cause the parent process to send a ParentProcessDidHandleProcessWasResumed IPC back, so that we can release our assertion on its behalf.
- // We are not using sendWithAsyncReply() because it would not be thread-safe.
- parentProcessConnection()->send(Messages::WebProcessProxy::ProcessWasResumed(), 0);
+ parentProcessConnection()->sendWithAsyncReply(Messages::WebProcessProxy::ProcessWasResumed(), [this] {
+ LockHolder holder(m_processWasResumedUIAssertionLock);
+ ASSERT(m_processWasResumedUIAssertion);
+ RELEASE_LOG(ProcessSuspension, "%p - WebProcess::parentProcessDidHandleProcessWasResumed() Releasing 'WebProcess was resumed' assertion on behalf on UIProcess", this);
+ [m_processWasResumedUIAssertion invalidate];
+ m_processWasResumedUIAssertion = nullptr;
+ });
}
-void WebProcess::parentProcessDidHandleProcessWasResumed()
-{
- LockHolder holder(m_processWasResumedUIAssertionLock);
- ASSERT(m_processWasResumedUIAssertion);
-
- RELEASE_LOG(ProcessSuspension, "%p - WebProcess::parentProcessDidHandleProcessWasResumed() Releasing 'WebProcess was resumed' assertion on behalf on UIProcess", this);
-
- [m_processWasResumedUIAssertion invalidate];
- m_processWasResumedUIAssertion = nullptr;
-}
-
#endif
#if PLATFORM(IOS_FAMILY)
