[webkit-changes] [WebKit/WebKit] 93d256: Crash under WorkerWorkerAgent::connectToAllWorkerI...

[Chris Dumez]

  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: 93d2569cbae15d0c282008b146e8e0ac0c3d737a
      https://github.com/WebKit/WebKit/commit/93d2569cbae15d0c282008b146e8e0ac0c3d737a
  Author: Chris Dumez <cdumez at apple.com>
  Date:   2025-01-23 (Thu, 23 Jan 2025)

  Changed paths:
    M Source/WebCore/SaferCPPExpectations/UncountedLocalVarsCheckerExpectations
    M Source/WebCore/inspector/agents/page/PageWorkerAgent.cpp
    M Source/WebCore/inspector/agents/worker/WorkerWorkerAgent.cpp
    M Source/WebCore/workers/WorkerInspectorProxy.cpp
    M Source/WebCore/workers/WorkerInspectorProxy.h
    M Source/WebCore/workers/WorkerMessagingProxy.cpp

  Log Message:
  -----------
  Crash under WorkerWorkerAgent::connectToAllWorkerInspectorProxies() on the bots
https://bugs.webkit.org/show_bug.cgi?id=286300

Reviewed by Devin Rousso.

WorkerWorkerAgent::connectToAllWorkerInspectorProxies() was getting called off the main thread
and iterating over the WeakHashSet returned by `WorkerInspectorProxy::allWorkerInspectorProxiesCopy()`.
The loop in connectToAllWorkerInspectorProxies() was ref'ing each WorkerInspectorProxy before checking
if the proxy is relevant for the current agent. This ref'ing was needed for safety since the proxy could
get destroyed concurrently on another thread otherwise. However, WorkerInspectorProxy is not
ThreadSafeRefCounted and we were using WeakPtr (instead of ThreadSafeWeakPtr). As a result, the ref'ing
of the object on another thread wasn't safe. Making WorkerInspectorProxy thread safe is not trivial since
it holds strings.

To make the code thread-safe, the global map of WorkerInspectorProxy now stores the context identifier
as key. When looking up the context identifier in the HashMap, we're holding a lock so we're safe on this
front. We then ref the proxies and add them to a Vector while holding the lock. The ref'ing is safe here
before we know we're only ref'ing the proxies for the right context and thus on the correct thread.

* Source/WebCore/inspector/agents/page/PageWorkerAgent.cpp:
(WebCore::PageWorkerAgent::connectToAllWorkerInspectorProxies):
* Source/WebCore/inspector/agents/worker/WorkerWorkerAgent.cpp:
(WebCore::WorkerWorkerAgent::connectToAllWorkerInspectorProxies):
* Source/WebCore/workers/WorkerInspectorProxy.cpp:
(WebCore::pageOrWorkerGlobalScopeIdentifier):
(WebCore::WTF_REQUIRES_LOCK):
(WebCore::WorkerInspectorProxy::forEachProxyInContext):
(WebCore::WorkerInspectorProxy::workerStarted):
(WebCore::WorkerInspectorProxy::workerTerminated):
(WebCore::WorkerInspectorProxy::allWorkerInspectorProxiesCopy): Deleted.
* Source/WebCore/workers/WorkerInspectorProxy.h:
* Source/WebCore/workers/WorkerMessagingProxy.cpp:
(WebCore::WorkerMessagingProxy::startWorkerGlobalScope):

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



To unsubscribe from these emails, change your notification settings at https://github.com/WebKit/WebKit/settings/notifications