Intermittent crash in imported/w3c/web-platform-tests/workers/semanti…

…cs/multiple-workers/exposure.any.html

https://bugs.webkit.org/show_bug.cgi?id=268802
rdar://122365041

Reviewed by Darin Adler.

In MessagePort::notifyMessageAvailable(), we were looking up the script execution
context identifier from the message port identifier in a first HashMap. Then we
would dispatch an asynchronous task to this script execution context's context
thread. In that task, we would look up the MessagePort from its identifier in
a second map.

Because the 2 HashMap lookups would happens at different times, releasing the lock
in between, we could end up in a case where: by the time the async task runs on
the context thread, a new MessagePort has been added to the map, with the same
identifier (sadly this can happen because MessagePorts can be sent across threads).
As a result, the script execution context of this new port may not match the original
context we dispatched too earlier. In the context of the crash, we would get:
- We're dealing with a worker MessagePort, so we dispatch to that worker thread
- On the worker thread, we look up the MessagePort and look up its script execution
  context. We expect its context to be a WorkerGlobalScope. However, it ends up being
  a Document (which we try to ref on a worker thread) since the MessagePort we looked
  up is a main thread MessagePort.

To address the issue, we now look up the MessagePort at the same time as the context
lookup, to make sure that they are consistent with each other. Then we capture this
port in the lambda we run on the scriptExecutionContext's context thread.

* Source/WebCore/dom/MessagePort.cpp:
(WebCore::MessagePort::notifyMessageAvailable):

Canonical link: https://commits.webkit.org/274218@main

Source/WebCore/dom/MessagePort.cpp

@@ -69,20 +69,17 @@ void MessagePort::notifyMessageAvailable(const MessagePortIdentifier& identifier
 {
     ASSERT(isMainThread());
     ScriptExecutionContextIdentifier scriptExecutionContextIdentifier;
+    ThreadSafeWeakPtr<MessagePort> weakPort;
     {
         Locker locker { allMessagePortsLock };
         scriptExecutionContextIdentifier = portToContextIdentifier().get(identifier);
+        weakPort = allMessagePorts().get(identifier);
     }
     if (!scriptExecutionContextIdentifier)
         return;
 
-    ScriptExecutionContext::ensureOnContextThread(scriptExecutionContextIdentifier, [identifier](auto&) {
-        RefPtr<MessagePort> port;
-        {
-            Locker locker { allMessagePortsLock };
-            port = allMessagePorts().get(identifier).get();
-        }
-        if (port)
+    ScriptExecutionContext::ensureOnContextThread(scriptExecutionContextIdentifier, [weakPort = WTFMove(weakPort)](auto&) {
+        if (RefPtr port = weakPort.get())
             port->messageAvailable();
     });
 }