fix: unified resource lifecycle — Release() via ResourceRef.Drop() (#287)

CHANGELOG.md

@@ -5,6 +5,23 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [0.30.28] - 2026-07-30
+
+### Fixed
+
+- **Resource lifecycle: BindGroup/Pipeline Release() bypasses ref-counting** —
+  `Release()` called `dq.Defer(lastSubmissionIndex)` directly, ignoring the
+  `ResourceRef` ref-counting system. On shared encoder path, this caused
+  use-after-free: HAL resource destroyed while GPU still processing commands.
+  Now `Release()` calls `ref.Drop()` — HAL destruction deferred until ALL
+  refs (user + GPU) are dropped. Matches Rust wgpu `Arc<BindGroup>` pattern.
+  Applied to: BindGroup, RenderPipeline, ComputePipeline. (ADR-056, #287)
+
+- **DestroyQueue deadlock: Triage → onZero → Defer re-entry** —
+  `Triage()` held mutex while executing callbacks. When `onZero` fired and
+  called `Defer()`, it tried to acquire the same mutex → deadlock. Now
+  callbacks execute outside the lock. Same fix applied to `FlushAll()`.
+
 ## [0.30.27] - 2026-07-30
 
 ### Fixed

bind_native.go

@@ -17,11 +17,9 @@ import (
 // the BindGroup itself — runtime.AddCleanup requires the callback argument to
 // be independent of the cleaned-up object.
 type bindGroupCleanupRef struct {
-	label        string
-	released     *atomic.Bool
-	destroyQueue *core.DestroyQueue
-	lastSubIdx   func() uint64
-	destroyFn    func()
+	label    string
+	released *atomic.Bool
+	ref      *core.ResourceRef // for ref.Drop() in GC path (ADR-056)
 }
 
 // BindGroupLayout defines the structure of resource bindings for shaders.
@@ -200,14 +198,15 @@ type BindGroup struct {
 	boundTextures []*Texture
 }
 
-// Release marks the bind group for destruction. The underlying HAL BindGroup
-// (and its descriptor heap slots) is not freed immediately — it is deferred via
-// DestroyQueue until the GPU completes any submission that may reference it.
-// This prevents descriptor use-after-free on DX12 with maxFramesInFlight=2
-// (BUG-DX12-007).
+// Release marks the bind group for destruction. Drops the application's
+// ownership reference via ResourceRef.Drop(). If the bind group is still
+// referenced by in-flight GPU submissions (Clone'd via SetBindGroup), the
+// HAL bind group stays alive until the GPU completes and Triage drops all
+// tracked refs. The onZero callback (set at CreateBindGroup) fires only
+// when the last reference drops, deferring HAL destruction via DestroyQueue.
 //
-// Matches Rust wgpu pattern: BindGroup::drop() only fires after
-// triage_submissions confirms fence completion.
+// This matches Rust wgpu's Arc<BindGroup> Drop behavior — deterministic,
+// refcount-driven destruction. ADR-056: unified resource lifecycle.
 func (g *BindGroup) Release() {
 	if g.released == nil || !g.released.CompareAndSwap(false, true) {
 		return
@@ -216,71 +215,26 @@ func (g *BindGroup) Release() {
 	// Cancel the GC cleanup — we are destroying explicitly.
 	g.cleanup.Stop()
 
-	if g.device == nil {
-		return
-	}
-
-	halDevice := g.device.halDevice()
-	if halDevice == nil {
-		return
+	if g.ref != nil {
+		g.ref.Drop()
 	}
-
-	dq := g.device.destroyQueue()
-	if dq == nil {
-		halDevice.DestroyBindGroup(g.hal)
-		return
-	}
-
-	subIdx := g.device.lastSubmissionIndex()
-	halBG := g.hal
-	dq.Defer(subIdx, "BindGroup", func() {
-		halDevice.DestroyBindGroup(halBG)
-	})
 }
 
 // registerBindGroupCleanup registers a runtime.AddCleanup handler on the bind group.
 // When GC collects the bind group without an explicit Release(), the cleanup
-// schedules deferred destruction via DestroyQueue — the same path as Release().
-func registerBindGroupCleanup(bg *BindGroup, dev *Device, label string) runtime.Cleanup {
-	halDevice := dev.halDevice()
-	if halDevice == nil {
-		// No HAL device — nothing to destroy.
-		return runtime.Cleanup{}
-	}
-
-	halBG := bg.hal
-	destroyFn := func() {
-		halDevice.DestroyBindGroup(halBG)
-	}
-
-	dq := dev.destroyQueue()
-	if dq == nil {
-		// No DestroyQueue — register cleanup that destroys immediately.
-		return runtime.AddCleanup(bg, func(ref bindGroupCleanupRef) {
-			if !ref.released.CompareAndSwap(false, true) {
-				return
-			}
-			slog.Warn("wgpu: BindGroup released by GC (missing explicit Release)", "label", ref.label)
-			ref.destroyFn()
-		}, bindGroupCleanupRef{
-			label:     label,
-			released:  bg.released,
-			destroyFn: destroyFn,
-		})
-	}
-
+// drops the ResourceRef — the same refcount-driven path as Release(). ADR-056.
+func registerBindGroupCleanup(bg *BindGroup, _ *Device, label string) runtime.Cleanup {
 	return runtime.AddCleanup(bg, func(ref bindGroupCleanupRef) {
 		if !ref.released.CompareAndSwap(false, true) {
 			return
 		}
 		slog.Warn("wgpu: BindGroup released by GC (missing explicit Release)", "label", ref.label)
-		subIdx := ref.lastSubIdx()
-		ref.destroyQueue.Defer(subIdx, "BindGroup(GC):"+ref.label, ref.destroyFn)
+		if ref.ref != nil {
+			ref.ref.Drop()
+		}
 	}, bindGroupCleanupRef{
-		label:        label,
-		released:     bg.released,
-		destroyQueue: dq,
-		lastSubIdx:   dev.lastSubmissionIndex,
-		destroyFn:    destroyFn,
+		label:    label,
+		released: bg.released,
+		ref:      bg.ref,
 	})
 }

core/destroy_queue.go

@@ -99,17 +99,24 @@ func (q *DestroyQueue) TrackSubmission(index uint64, refs []*ResourceRef) {
 //
 // Also Drop()'s ResourceRefs from completed TrackedSubmissions (Phase 2).
 //
+// Callbacks and ref Drop()'s are executed OUTSIDE the mutex to prevent deadlocks
+// when an onZero callback re-enters the DestroyQueue (ADR-056: ref.Drop() ->
+// onZero -> dq.Defer() must not deadlock with Triage holding the lock).
+//
 // This should be called after each Queue.Submit() with the result of
 // hal.Queue.PollCompleted().
 func (q *DestroyQueue) Triage(completedIndex uint64) {
+	// Collect completed items under the lock; execute callbacks outside.
+	var completedDestroys []func()
+	var completedRefs []*ResourceRef
+
 	q.mu.Lock()
-	defer q.mu.Unlock()
 
 	// Triage deferred destroys (Phase 1).
 	n := 0
 	for i := range q.pending {
 		if q.pending[i].submissionIndex <= completedIndex {
-			q.pending[i].destroyFn()
+			completedDestroys = append(completedDestroys, q.pending[i].destroyFn)
 		} else {
 			q.pending[n] = q.pending[i]
 			n++
@@ -125,9 +132,7 @@ func (q *DestroyQueue) Triage(completedIndex uint64) {
 	tn := 0
 	for i := range q.tracked {
 		if q.tracked[i].index <= completedIndex {
-			for _, ref := range q.tracked[i].refs {
-				ref.Drop()
-			}
+			completedRefs = append(completedRefs, q.tracked[i].refs...)
 		} else {
 			q.tracked[tn] = q.tracked[i]
 			tn++
@@ -137,26 +142,49 @@ func (q *DestroyQueue) Triage(completedIndex uint64) {
 		q.tracked[i] = TrackedSubmission{}
 	}
 	q.tracked = q.tracked[:tn]
+
+	q.mu.Unlock()
+
+	// Execute Phase 1 destroy callbacks outside the lock.
+	for _, fn := range completedDestroys {
+		fn()
+	}
+
+	// Drop Phase 2 refs outside the lock.
+	// onZero callbacks may call q.Defer() — safe because the lock is released.
+	for _, ref := range completedRefs {
+		ref.Drop()
+	}
 }
 
 // FlushAll destroys all pending resources regardless of GPU completion status.
 // Called during device shutdown when all GPU work is (or should be) complete.
 // Also Drop()'s all tracked submission refs (Phase 2).
+//
+// Callbacks and ref Drop()'s are executed outside the mutex to prevent deadlocks
+// when an onZero callback re-enters the DestroyQueue (ADR-056).
 func (q *DestroyQueue) FlushAll() {
 	q.mu.Lock()
-	defer q.mu.Unlock()
 
-	for i := range q.pending {
-		q.pending[i].destroyFn()
-	}
+	// Take ownership of slices and clear under the lock.
+	destroys := q.pending
+	tracked := q.tracked
 	q.pending = nil
+	q.tracked = nil
 
-	for i := range q.tracked {
-		for _, ref := range q.tracked[i].refs {
+	q.mu.Unlock()
+
+	// Execute Phase 1 destroy callbacks outside the lock.
+	for i := range destroys {
+		destroys[i].destroyFn()
+	}
+
+	// Drop Phase 2 refs outside the lock.
+	for i := range tracked {
+		for _, ref := range tracked[i].refs {
 			ref.Drop()
 		}
 	}
-	q.tracked = nil
 }
 
 // Len returns the number of pending deferred destructions. For testing only.

device_native.go

@@ -437,13 +437,30 @@ func (d *Device) CreateBindGroup(desc *BindGroupDescriptor) (*BindGroup, error)
 	// Collect buffer and texture references for submit-time validation (VAL-A6).
 	boundBuffers, boundTextures := collectBindGroupResources(desc.Entries)
 
+	// Initialize ResourceRef with onZero callback for refcount-driven destruction.
+	// When the last reference drops (either from explicit Release or Phase 2
+	// Triage after GPU completion), onZero fires and defers HAL destruction via
+	// DestroyQueue. This matches Rust wgpu's Arc<BindGroup> Drop behavior. ADR-056.
+	halBG := halGroup
+	bgOnZero := func() {
+		dq := d.destroyQueue()
+		if dq != nil {
+			subIdx := d.lastSubmissionIndex()
+			dq.Defer(subIdx, "BindGroup", func() {
+				halDevice.DestroyBindGroup(halBG)
+			})
+		} else {
+			halDevice.DestroyBindGroup(halBG)
+		}
+	}
+
 	bg := &BindGroup{
 		hal:                    halGroup,
 		device:                 d,
 		released:               new(atomic.Bool),
 		layout:                 desc.Layout,
 		lateBufferBindingInfos: lateInfos,
-		ref:                    core.NewResourceRef("BindGroup:"+desc.Label, nil),
+		ref:                    core.NewResourceRef("BindGroup:"+desc.Label, bgOnZero),
 		boundBuffers:           boundBuffers,
 		boundTextures:          boundTextures,
 	}
@@ -538,6 +555,23 @@ func (d *Device) CreateRenderPipeline(desc *RenderPipelineDescriptor) (*RenderPi
 
 	lateGroups := makeLateSizedBufferGroups(shaderBindingSizes, bgLayouts)
 
+	// Initialize ResourceRef with onZero callback for refcount-driven destruction.
+	// When the last reference drops (either from explicit Release or Phase 2
+	// Triage after GPU completion), onZero fires and defers HAL destruction via
+	// DestroyQueue. ADR-056: unified resource lifecycle.
+	halRP := halPipeline
+	rpOnZero := func() {
+		dq := d.destroyQueue()
+		if dq != nil {
+			subIdx := d.lastSubmissionIndex()
+			dq.Defer(subIdx, "RenderPipeline", func() {
+				halDevice.DestroyRenderPipeline(halRP)
+			})
+		} else {
+			halDevice.DestroyRenderPipeline(halRP)
+		}
+	}
+
 	return &RenderPipeline{
 		hal:                   halPipeline,
 		device:                d,
@@ -547,7 +581,7 @@ func (d *Device) CreateRenderPipeline(desc *RenderPipelineDescriptor) (*RenderPi
 		blendConstantRequired: needsBlendConstant,
 		stripIndexFormat:      desc.Primitive.StripIndexFormat,
 		lateSizedBufferGroups: lateGroups,
-		ref:                   core.NewResourceRef("RenderPipeline:"+desc.Label, nil),
+		ref:                   core.NewResourceRef("RenderPipeline:"+desc.Label, rpOnZero),
 	}, nil
 }
 
@@ -633,13 +667,28 @@ func (d *Device) CreateComputePipeline(desc *ComputePipelineDescriptor) (*Comput
 
 	lateGroups := makeLateSizedBufferGroups(shaderBindingSizes, bgLayouts)
 
+	// Initialize ResourceRef with onZero callback for refcount-driven destruction.
+	// ADR-056: unified resource lifecycle.
+	halCP := halPipeline
+	cpOnZero := func() {
+		dq := d.destroyQueue()
+		if dq != nil {
+			subIdx := d.lastSubmissionIndex()
+			dq.Defer(subIdx, "ComputePipeline", func() {
+				halDevice.DestroyComputePipeline(halCP)
+			})
+		} else {
+			halDevice.DestroyComputePipeline(halCP)
+		}
+	}
+
 	return &ComputePipeline{
 		hal:                   halPipeline,
 		device:                d,
 		bindGroupCount:        bgCount,
 		bindGroupLayouts:      bgLayouts,
 		lateSizedBufferGroups: lateGroups,
-		ref:                   core.NewResourceRef("ComputePipeline:"+desc.Label, nil),
+		ref:                   core.NewResourceRef("ComputePipeline:"+desc.Label, cpOnZero),
 	}, nil
 }