[webkit-changes] [WebKit/WebKit] 33766a: Cherry-pick 289288 at main (93d2569cbae1). https://bu...

[Chris Dumez]

  Branch: refs/heads/webkitglib/2.46
  Home:   https://github.com/WebKit/WebKit
  Commit: 33766a2ff194a355c8913360ced162d48547d7d6
      https://github.com/WebKit/WebKit/commit/33766a2ff194a355c8913360ced162d48547d7d6
  Author: Chris Dumez <cdumez at apple.com>
  Date:   2025-01-29 (Wed, 29 Jan 2025)

  Changed paths:
    M Source/WebCore/SmartPointerExpectations/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:
  -----------
  Cherry-pick 289288 at main (93d2569cbae1). https://bugs.webkit.org/show_bug.cgi?id=286300

    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

Canonical link: https://commits.webkit.org/282416.409@webkitglib/2.46



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