[5] Committed CommandBuffer destruction thread decided by release ordering

MediumWebGPU Metal backendRace

8783151

Medium. The change trades an explicit owner and a release-time drain for two internal strong captures, so which of them performs the final release decides which thread runs ~CommandBuffer(). Only some of the possible orderings put that thread where a non-atomic child refcount and the device's encoder map get touched concurrently.

WebKit's WebGPU implementation runs in the GPU process, driven by IPC messages from WebContent, and hands recorded command buffers to Metal, which calls back on a thread of its own choosing when the GPU finishes with them. A CommandBuffer — the committed unit of GPU work — uses thread-safe reference counting, while the CommandEncoder it holds a reference to, the object that recorded those commands, derives from RefCountedAndCanMakeWeakPtr and so uses a plain non-atomic counter. That pairing is safe only as long as the last release of a command buffer happens on a thread allowed to touch the encoder's counter and the device's encoder map.

The angle: if the final release lands on Metal's completion thread, a non-atomic refcount and the device's encoder map are mutated concurrently with the thread that services WebGPU IPC — the shape that produces torn counts and premature destruction.

Patch Details

CommandBuffer::makeInvalidDueToCommit() no longer hands a Ref<CommandBuffer> to Instance::retainCommandBuffer(), so the Instance-owned container m_retainedCommandBufferInstances no longer pins committed buffers past Metal's completion callback. wgpuInstanceRelease's waitForCommandBufferCompletions() drain of everything still in flight is removed with it. In their place, the Queue::scheduleWork continuation gets a strong protectedThis copy back — it previously captured only a ThreadSafeWeakPtr.

Ownership handed to whichever of several internal references happens to release last, so the destruction thread of a cross-thread object becomes a property of timing rather than of design.

  Ordering A (benign)                 Ordering B (hazardous)
  -------------------                 ----------------------
  Metal releases the block            the work item runs and is
  while the work item is still        destroyed before Metal
  pending                             releases the block
        |                                   |
  the work item holds the last Ref    the block release performs
        |                             the final deref()
  ~CommandBuffer() runs on the              |
  work item's thread                  ~CommandBuffer() runs on
                                      Metal's completion thread
                                            |
                                      non-atomic CommandEncoder
                                      deref + Device map mutation
                                      race the IPC-servicing thread

Background

Where this lives. Source/WebGPU/WebGPU/ is the native implementation behind the WebGPU IPC surface, driven from WebContent via RemoteDevice, RemoteCommandEncoder, RemoteCommandBuffer and RemoteQueue messages. CommandBuffer, CommandEncoder, Device and Instance are its object-lifetime core.

Metal completion handlers. Metal invokes addCompletedHandler: blocks on its own internal thread. A block's captures survive until the framework releases the block, which is a separate event from the handler returning.

Two refcount flavours. CommandBuffer derives from ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr; CommandEncoder derives from RefCountedAndCanMakeWeakPtr (CommandEncoder.h), a plain non-atomic counter. The distinction determines which threads may legally perform ref()/deref() on each.

The device's encoder map. Device::createCommandEncoder() writes m_commandEncoderMap.set(commandEncoder->uniqueId(), commandEncoder.ptr()) on whichever thread services RemoteDevice IPC messages, and Device::removeCommandEncoder(m_uniqueId) erases from the same map. The map has no lock.

Analysis

After the change, makeInvalidDueToCommit() creates two backend-internal strong references to a committed CommandBuffer: the one captured by value into the addCompletedHandler: block (protectedThis = protect(*this)), and the copy that block places into the scheduled work item. Other strong references exist for a while — Queue::submit takes Vector<Ref<CommandBuffer>>, and the WebKit-side remote wrapper very likely holds its own reference until the Destruct message, though that wrapper lives on the WebKit side of the IPC boundary, outside this backend. Once those have released, the last release comes from one of the two internal captures, and which one determines the destruction thread.

That restored strong capture is itself a lifetime anchor and substitutes for part of what the removed pin provided — this is a partial replacement, not a bare removal. When Metal releases an addCompletedHandler: block relative to the handler's return is a framework-internal detail, which is what leaves both orderings in the diagram live. In ordering A the work item holds the last reference and ~CommandBuffer() would run on the thread that executes or destroys it. Ordering B is the residual hazard: if the work item is executed and destroyed before Metal releases the block, or if the schedule-work implementation dispatches the item synchronously on the completion thread, Metal's release would perform the final deref().

A second path opens at teardown, and this one has no replacement at all. With waitForCommandBufferCompletions() gone, wgpuInstanceRelease no longer waits for in-flight buffers, so if pending work items holding Ref<CommandBuffer> are destroyed as part of ~Instance() — the Instance::defaultScheduleWork fallback appends them to m_pendingWork — the final release would land on whichever thread released the instance.

The destructor is where the invariant would break. ~CommandBuffer() calls retainTimestampsForOneUpdateLoop(), which copies m_commandEncoder into a local RefPtr, and then destroys the RefPtr<CommandEncoder> member. Those ref()/deref() operations on a non-atomic counter would race any concurrent traffic on the same counter. If the count reached zero off-thread, ~CommandEncoder() would run there and call m_device->removeCommandEncoder(m_uniqueId), mutating Device::m_commandEncoderMap concurrently with createCommandEncoder(). Which thread services RemoteDevice IPC messages is not the load-bearing point — only that it is a different thread from Metal's completion thread, which is what makes the map mutation concurrent. ~CommandEncoder() additionally calls finalizeBlitCommandEncoder() and clearTracking(), both of which touch Metal encoder state and ObjC collections with no synchronization.

This commit does not introduce that hazard; it restores an ownership shape in which the destruction thread of a committed CommandBuffer is decided by release ordering rather than pinned by an explicit owner and a release-time drain. The outcome in the bad orderings is non-atomic refcount corruption — premature destruction or use-after-free — plus unsynchronized hash-table mutation in the GPU process.

Audit directions

Thread-safe parents carrying non-thread-safe children. A ThreadSafeRefCounted object that holds a RefPtr to a plain RefCounted object exports its own thread-freedom onto a child that cannot support it, and the violation only materialises in the destructor. Enumerate the members of the ThreadSafeRefCounted types in Source/WebGPU/WebGPU/ and flag every RefPtr<T> whose T derives from plain RefCounted. The review tell is a class declaration line naming a thread-safe base whose member list contains a non-thread-safe smart pointer.
Destructors that mutate shared containers. removeCommandEncoder is reachable only from ~CommandEncoder(), so the thread that runs the destructor is the thread that mutates the map — which makes the map's thread-safety a property of every release site rather than of the map's own code. Audit the other Device-owned registries for erase calls reachable from a destructor, and ask for each which threads can hold the last reference.
Removed drains at teardown. A wait-for-completion call at instance release is often the only thing making an otherwise timing-dependent ownership graph deterministic. Where such a drain is removed, the forward-facing question is which pending containers can now be destroyed with in-flight work still referenced; start at Instance::defaultScheduleWork and m_pendingWork.