[3] Cross-thread destruction of WebGPU CommandBuffer on Metal's completion queue

HighWebGPU GPU ProcessRace

A queue that quietly declines work picks the thread your destructor runs on.

70e05bc

High. Two threads perform non-atomic read-modify-writes on the same refcount word, so a lost update either drives the count to zero with a live holder outstanding or lets deref() reach zero twice. The race window opens specifically during RemoteGPU teardown, which a renderer drives directly.

Cross-thread reference counting in WebKit's GPU-process graphics stack is normally kept safe by a single-owner rule: an object whose refcount is non-atomic is only ever touched by the thread that owns it. WebGPU's Metal backend runs its work on a single per-Instance thread — an Instance being the per-RemoteGPU root object that services WebGPU IPC from the WebContent process — while Metal itself calls completion handlers back on a dispatch queue of its own, on a thread WebGPU does not own. The bridge between the two is a work-queue hop: the Metal-thread block's only job is to bounce real cleanup back to the owning thread, and the whole design rests on that hop always completing.

The angle: a page driving WebGPU can tear down the GPU connection at the moment a command buffer completes and have the last reference drop on Metal's thread, tearing a non-atomic refcount shared with the WebGPU work-queue thread.

The Metal-thread entry point and its strong capture:

Source/WebGPU/WebGPU/CommandBuffer.mm

// CommandBuffer::makeInvalidDueToCommit() registers a Metal completion
// handler capturing protect(*this) — a strong Ref<CommandBuffer> — whose
// body posts an inner lambda to Queue::scheduleWork carrying the same
// strong Ref.

The non-atomic member whose count is torn:

Source/WebGPU/WebGPU/CommandEncoder.h

class CommandEncoder : public CommandsMixin, public RefCountedAndCanMakeWeakPtr<CommandEncoder>

Patch Details

The change restores the single-owner invariant three ways. Instance::retainCommandBuffer parks a Ref<CommandBuffer> in a new m_retainedCommandBufferInstances container, keyed by a WeakObjCPtr<id<MTLCommandBuffer>> and lazily pruned once that pointer goes nil, so the Metal-thread block's deref can never be the last one. wgpuInstanceRelease gains a call to waitForCommandBufferCompletions() — [commandBuffer waitUntilCompleted] on everything committed — before its deref(), keeping the C-API reference alive across handler execution. And the inner posted lambda's capture is demoted from a strong Ref<CommandBuffer> to a ThreadSafeWeakPtr<CommandBuffer>.

Object anchored by a work item that the queue is permitted to silently drop, making an unowned thread the site of last-reference release.

Background

Where this lives. Source/WebGPU/WebGPU/ is the Metal-backed WebGPU implementation inside the GPU process. It services RemoteGPU/RemoteDevice/RemoteQueue IPC from WebContent. CommandBuffer and CommandEncoder wrap id<MTLCommandBuffer> and the encoded command state respectively.

Threading model. Work normally executes on a single per-Instance StreamConnectionWorkQueue thread. Metal, however, invokes addCompletedHandler:/addScheduledHandler: blocks on its own dispatch queue, a thread WebGPU does not own. Queue::scheduleWork is the hop that gets work from Metal's thread back onto the owned one.

Atomic versus non-atomic refcounts. WebKit has two families. ThreadSafeRefCounted uses atomic increments and decrements and is safe under concurrent access; plain RefCounted does not, and a concurrent read-modify-write on its counter is a data race in the strict sense — two threads can read the same pre-value and both write back the same decremented result, losing one update entirely. CommandBuffer is ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr<CommandBuffer>; its members are not uniformly so.

Work-item destruction. A lambda submitted to a queue that declines to run it is destroyed in place, on whatever thread called dispatch. Its captures are released there. This is ordinary C++ behavior, but it is the load-bearing fact: a dropped work item releases its captures on the submitting thread, not the intended target thread.

Analysis

The bug is a cross-thread destruction race producing a torn non-atomic reference count — a use-after-free, with a secondary unsynchronised container-mutation race alongside it.

  Metal completion thread          WebGPU work-queue thread
  ──────────────────────           ────────────────────────
  completion handler fires         RemoteGPU teardown begins
  Queue::scheduleWork(lambda)      m_shouldQuit = true
    └─ dispatch() drops item       ObjectHeap released
       lambda destroyed in place   (RemoteCommandBuffer_Destruct)
       last Ref<CommandBuffer>            │
       ~CommandBuffer() here             │
         └─ ~RefPtr<CommandEncoder>      │
              non-atomic deref ──────────┼──► non-atomic deref
                                         ▼    on the same word

Per the commit message, StreamConnectionWorkQueue::dispatch silently drops the submitted work item once m_shouldQuit is set. The dropped lambda is destroyed on the Metal thread along with the strong Ref<CommandBuffer> it captured, and once the GPU-process ObjectHeap entries are released by RemoteCommandBuffer_Destruct/RemoteCommandEncoder_Destruct, that drop is the last reference. ~CommandBuffer then runs on the Metal thread.

CommandBuffer's own count is atomic, so that part survives. The damage is in its members: CommandBuffer.h declares RefPtr<CommandEncoder> m_commandEncoder, and CommandEncoder is RefCountedAndCanMakeWeakPtr — non-atomic. ~CommandBuffer() destroys that member as ordinary member teardown, performing a non-atomic decrement on the Metal thread while the work-queue thread concurrently decrements the same counter as the ObjectHeap clears. A lost decrement leaks; a lost increment, or an interleaving where both threads observe the same pre-value and both write back the decremented one, drives the count to zero while another holder still has a pointer, or lets deref() reach zero twice.

The commit message adds a second leg: ~CommandEncoder mutates the unlocked Device::m_commandEncoderMap, racing the work-queue thread's concurrent removeCommandEncoder. Concurrent mutation of a HashMap with no lock can corrupt the table independently of the refcount race.

The same seam has a companion path. Every WebGPU addCompletedHandler/addScheduledHandler that reaches Queue::scheduleWork materialises a temporary RefPtr<Instance> from a ThreadSafeWeakPtr on the Metal thread — Device::instance() in Device.h returns RefPtr<Instance> built from m_instance.get(). If that temporary outlives the work-queue thread's wgpuInstanceRelease deref, ~Instance runs on the Metal thread and destroys the Ref<Device> keys of retainedDeviceInstances there.

The third part of the fix is load-bearing rather than belt-and-braces, and the commit message says so explicitly: Instance::scheduleWork wraps the work item with makeBlockPtr(WTF::move(workItem)).get(), and that ObjC wrapper is autoreleased on the Metal thread, draining after waitUntilCompleted returns. A strong inner capture would outlive the anchor that waitForCommandBufferCompletions() provides, so the ThreadSafeWeakPtr demotion is required for the anchor to actually bound the lifetime.

This vulnerability weakens GPU-process memory safety on a surface a renderer reaches through ordinary WebGPU usage plus connection teardown, which the renderer controls.

Audit directions

Work queues that drop items during teardown. Any queue whose dispatch can return without running the item makes the submitting thread the destruction site for that item's captures. StreamConnectionWorkQueue::dispatch under m_shouldQuit is the instance here; the audit target is every other WebKit work queue with a quit flag, checked against callers that capture strong references intending them to be released on the target thread. The review tell is a dispatch() call whose return value is ignored in a function whose comment or structure implies "this will run later."
Atomic outer object, non-atomic members. ThreadSafeRefCounted on a type says nothing about the refcount discipline of what it owns. CommandBuffer was thread-safe while holding a RefPtr<CommandEncoder> that was not. Sweep other ThreadSafeRefCounted graphics types for plain RefCounted members reachable from their destructors — the tell in review is a ThreadSafeRefCounted class declaration whose member list contains RefPtr<T> where T's own base is not thread-safe.
Unlocked containers mutated from destructors. ~CommandEncoder touching Device::m_commandEncoderMap with no lock is safe only while destruction is thread-confined. Wherever a destructor de-registers from a shared map, check whether thread confinement is actually enforced or merely assumed.