Diff
Modified: branches/safari-605-branch/Source/WebCore/ChangeLog (229553 => 229554)
--- branches/safari-605-branch/Source/WebCore/ChangeLog 2018-03-12 21:59:28 UTC (rev 229553)
+++ branches/safari-605-branch/Source/WebCore/ChangeLog 2018-03-12 21:59:32 UTC (rev 229554)
@@ -1,5 +1,29 @@
2018-03-11 Jason Marcell <jmarc...@apple.com>
+ Cherry-pick r229028. rdar://problem/37992284
+
+ 2018-02-26 Youenn Fablet <you...@apple.com>
+
+ MessagePort is not always destroyed in the right thread
+ https://bugs.webkit.org/show_bug.cgi?id=183053
+
+ Reviewed by Chris Dumez.
+
+ Make existingMessagePortForIdentifier take a lambda so that we hold the lock until there
+ is no longer a need to keep the MessagePort around.
+ This is very time sensitive and does not happen a lot when running WPT tests.
+
+ Update existing call sites to pass a lambda.
+
+ * dom/MessagePort.cpp:
+ (WebCore::MessagePort::existingMessagePortForIdentifier):
+ * dom/MessagePort.h:
+ * dom/messageports/MessagePortChannelProviderImpl.cpp:
+ (WebCore::MessagePortChannelProviderImpl::postMessageToRemote):
+ (WebCore::MessagePortChannelProviderImpl::checkProcessLocalPortForActivity):
+
+2018-03-11 Jason Marcell <jmarc...@apple.com>
+
Cherry-pick r228279. rdar://problem/38154561
2018-02-08 Chris Fleizach <cfleiz...@apple.com>
Modified: branches/safari-605-branch/Source/WebCore/dom/MessagePort.cpp (229553 => 229554)
--- branches/safari-605-branch/Source/WebCore/dom/MessagePort.cpp 2018-03-12 21:59:28 UTC (rev 229553)
+++ branches/safari-605-branch/Source/WebCore/dom/MessagePort.cpp 2018-03-12 21:59:32 UTC (rev 229554)
@@ -56,12 +56,8 @@
void MessagePort::deref() const
{
- // MessagePort::existingMessagePortForIdentifier() is unique in that it holds a raw pointer to a MessagePort
- // but might create a RefPtr from it.
- // If that happens on one thread at the same time that a MessagePort is being deref'ed and destroyed on a
- // different thread then Bad Things could happen.
- // This custom deref() function is designed to handle that contention by guaranteeing that nobody can be
- // creating a RefPtr inside existingMessagePortForIdentifier while the object is mid-deletion.
+ // This custom deref() function ensures that as long as the lock to allMessagePortsLock is taken, no MessagePort will be destroyed.
+ // This allows isExistingMessagePortLocallyReachable and notifyMessageAvailable to easily query the map and manipulate MessagePort instances.
if (!--m_refCount) {
Locker<Lock> locker(allMessagePortsLock());
@@ -74,11 +70,19 @@
}
}
-RefPtr<MessagePort> MessagePort::existingMessagePortForIdentifier(const MessagePortIdentifier& identifier)
+bool MessagePort::isExistingMessagePortLocallyReachable(const MessagePortIdentifier& identifier)
{
Locker<Lock> locker(allMessagePortsLock());
+ auto* port = allMessagePorts().get(identifier);
+ return port && port->isLocallyReachable();
+}
- return allMessagePorts().get(identifier);
+void MessagePort::notifyMessageAvailable(const MessagePortIdentifier& identifier)
+{
+ Locker<Lock> locker(allMessagePortsLock());
+ if (auto* port = allMessagePorts().get(identifier))
+ port->messageAvailable();
+
}
Ref<MessagePort> MessagePort::create(ScriptExecutionContext& scriptExecutionContext, const MessagePortIdentifier& local, const MessagePortIdentifier& remote)
Modified: branches/safari-605-branch/Source/WebCore/dom/MessagePort.h (229553 => 229554)
--- branches/safari-605-branch/Source/WebCore/dom/MessagePort.h 2018-03-12 21:59:28 UTC (rev 229553)
+++ branches/safari-605-branch/Source/WebCore/dom/MessagePort.h 2018-03-12 21:59:32 UTC (rev 229554)
@@ -57,8 +57,10 @@
// Returns nullptr if the passed-in vector is empty.
static ExceptionOr<TransferredMessagePortArray> disentanglePorts(Vector<RefPtr<MessagePort>>&&);
static Vector<RefPtr<MessagePort>> entanglePorts(ScriptExecutionContext&, TransferredMessagePortArray&&);
- WEBCORE_EXPORT static RefPtr<MessagePort> existingMessagePortForIdentifier(const MessagePortIdentifier&);
+ WEBCORE_EXPORT static bool isExistingMessagePortLocallyReachable(const MessagePortIdentifier&);
+ WEBCORE_EXPORT static void notifyMessageAvailable(const MessagePortIdentifier&);
+
WEBCORE_EXPORT void messageAvailable();
bool started() const { return m_started; }
bool closed() const { return m_closed; }
Modified: branches/safari-605-branch/Source/WebCore/dom/messageports/MessagePortChannelProviderImpl.cpp (229553 => 229554)
--- branches/safari-605-branch/Source/WebCore/dom/messageports/MessagePortChannelProviderImpl.cpp 2018-03-12 21:59:28 UTC (rev 229553)
+++ branches/safari-605-branch/Source/WebCore/dom/messageports/MessagePortChannelProviderImpl.cpp 2018-03-12 21:59:32 UTC (rev 229554)
@@ -82,11 +82,8 @@
void MessagePortChannelProviderImpl::postMessageToRemote(MessageWithMessagePorts&& message, const MessagePortIdentifier& remoteTarget)
{
performActionOnMainThread([registry = &m_registry, message = WTFMove(message), remoteTarget]() mutable {
- bool wasFirstMessageInQueue = registry->didPostMessageToRemote(WTFMove(message), remoteTarget);
- if (wasFirstMessageInQueue) {
- if (auto remotePort = MessagePort::existingMessagePortForIdentifier(remoteTarget))
- remotePort->messageAvailable();
- }
+ if (registry->didPostMessageToRemote(WTFMove(message), remoteTarget))
+ MessagePort::notifyMessageAvailable(remoteTarget);
});
}
@@ -119,13 +116,7 @@
{
ASSERT(isMainThread());
- auto port = MessagePort::existingMessagePortForIdentifier(identifier);
- if (!port) {
- callback(MessagePortChannelProvider::HasActivity::No);
- return;
- }
-
- callback(port->isLocallyReachable() ? HasActivity::Yes : HasActivity::No);
+ callback(MessagePort::isExistingMessagePortLocallyReachable(identifier) ? HasActivity::Yes : HasActivity::No);
}
Modified: branches/safari-605-branch/Source/WebKit/ChangeLog (229553 => 229554)
--- branches/safari-605-branch/Source/WebKit/ChangeLog 2018-03-12 21:59:28 UTC (rev 229553)
+++ branches/safari-605-branch/Source/WebKit/ChangeLog 2018-03-12 21:59:32 UTC (rev 229554)
@@ -1,3 +1,21 @@
+2018-03-11 Jason Marcell <jmarc...@apple.com>
+
+ Cherry-pick r229028. rdar://problem/37992284
+
+ 2018-02-26 Youenn Fablet <you...@apple.com>
+
+ MessagePort is not always destroyed in the right thread
+ https://bugs.webkit.org/show_bug.cgi?id=183053
+
+ Reviewed by Chris Dumez.
+
+ Update code to pass a lambda to MessagePort::existingMessagePortForIdentifier.
+
+ * WebProcess/WebCoreSupport/WebMessagePortChannelProvider.cpp:
+ (WebKit::WebMessagePortChannelProvider::checkProcessLocalPortForActivity):
+ * WebProcess/WebProcess.cpp:
+ (WebKit::WebProcess::messagesAvailableForPort):
+
2018-02-26 Jason Marcell <jmarc...@apple.com>
Cherry-pick r229037. rdar://problem/37912529
Modified: branches/safari-605-branch/Source/WebKit/WebProcess/WebCoreSupport/WebMessagePortChannelProvider.cpp (229553 => 229554)
--- branches/safari-605-branch/Source/WebKit/WebProcess/WebCoreSupport/WebMessagePortChannelProvider.cpp 2018-03-12 21:59:28 UTC (rev 229553)
+++ branches/safari-605-branch/Source/WebKit/WebProcess/WebCoreSupport/WebMessagePortChannelProvider.cpp 2018-03-12 21:59:32 UTC (rev 229554)
@@ -120,8 +120,7 @@
void WebMessagePortChannelProvider::checkProcessLocalPortForActivity(const MessagePortIdentifier& identifier, uint64_t callbackIdentifier)
{
- auto port = MessagePort::existingMessagePortForIdentifier(identifier);
- WebProcess::singleton().parentProcessConnection()->send(Messages::WebProcessProxy::DidCheckProcessLocalPortForActivity(callbackIdentifier, port && port->isLocallyReachable()), 0);
+ WebProcess::singleton().parentProcessConnection()->send(Messages::WebProcessProxy::DidCheckProcessLocalPortForActivity { callbackIdentifier, MessagePort::isExistingMessagePortLocallyReachable(identifier) }, 0);
}
void WebMessagePortChannelProvider::checkProcessLocalPortForActivity(const MessagePortIdentifier&, ProcessIdentifier, CompletionHandler<void(HasActivity)>&&)
Modified: branches/safari-605-branch/Source/WebKit/WebProcess/WebProcess.cpp (229553 => 229554)
--- branches/safari-605-branch/Source/WebKit/WebProcess/WebProcess.cpp 2018-03-12 21:59:28 UTC (rev 229553)
+++ branches/safari-605-branch/Source/WebKit/WebProcess/WebProcess.cpp 2018-03-12 21:59:32 UTC (rev 229554)
@@ -1070,9 +1070,7 @@
void WebProcess::messagesAvailableForPort(const MessagePortIdentifier& identifier)
{
- auto port = MessagePort::existingMessagePortForIdentifier(identifier);
- if (port)
- port->messageAvailable();
+ MessagePort::notifyMessageAvailable(identifier);
}
#if ENABLE(GAMEPAD)
