Snapshot Queue.All under the lock instead of holding RLock across the yield callback

[PratikDhanave]

What

Queue.All in internal/concurrent/queue.go previously held q.mu.RLock for the entire iteration and invoked each yield(item) (arbitrary caller code) while the read lock was still held. This change takes a snapshot of the items under the lock, releases the lock, and then yields the copied values.

Why

The method's doc states that All "corresponds to a consistent snapshot of the Queue's contents at the time iteration starts", but the implementation took no copy and kept the lock live across the callback. Because sync.RWMutex is non-reentrant:

• Any loop body that calls Enqueue, Dequeue, or Len on the same queue deadlocks.
• A slow consumer inside the loop blocks all writers for the duration of iteration.

Snapshotting under the lock and yielding afterwards matches the documented moment-in-time semantics and removes both hazards. This aligns with the enumeration semantics used across the .NET and Python SDKs, where enumerating a concurrent collection iterates a point-in-time view rather than pinning the collection's lock across user code. The sibling Map.All explicitly documents that it is not a snapshot; Queue.All documents that it is, so the implementation should honor that contract.

Tests

Added TestQueue_AllSnapshotAllowsMutationInLoop in the canonical queue_test.go. It enqueues items, then calls Enqueue/Dequeue on the same queue from inside for v := range q.All(), asserting under a timeout that iteration completes and yields exactly the original snapshot values. The test deadlocks (times out) against the previous code and passes after the fix. Verified with go test -race ./internal/concurrent/....

[Copilot]

Pull request overview

This PR updates Queue.All to honor its documented “consistent snapshot at iteration start” contract by copying the queue’s contents under RLock, then releasing the lock before invoking the caller’s yield callback—eliminating lock retention across arbitrary user code and preventing writer starvation / lock-order deadlocks.

Changes:

• Change Queue.All to snapshot q.items under RLock and iterate the snapshot after unlocking.
• Add a regression test that mutates the same queue inside for v := range q.All() and asserts no deadlock and stable snapshot values.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
internal/concurrent/queue.go	Implements snapshot iteration for Queue.All by copying under RLock and yielding after unlocking.
internal/concurrent/queue_test.go	Adds a timeout-based regression test ensuring All allows reentrant mutation in the loop body and yields only the original snapshot.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[github-actions]

Go API Parity Review

Result: ✅ Out of scope — no parity issues

This PR only modifies internal/concurrent/queue.go (and its test), fixing Queue.All to snapshot items under the lock rather than holding the read lock across the yield callback. Both changed files are in the internal/ package tree.

Since no exported Go APIs or user-visible behaviors are changed, this PR is outside the scope of cross-repo consistency review. No comparison with the upstream .NET or Python implementations is necessary.

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

• awmgmcpg

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "awmgmcpg"

See Network Configuration for more information.

Generated by Go API Consistency Review Agent · 15.1 AIC · ⌖ 3.91 AIC · ⊞ 5.7K · ◷