fix(core): hold map_state across the HAL map in Buffer::map

[AdrianEddy]

Connections

None.

Description

Buffer::unmap can fail with BufferAccessError::NotMapped for a buffer that is simply mid-map, when map and unmap run on different threads.

Buffer::map took the map_state lock as a temporary to swap the state out, ran the HAL map, then re-locked to store Active. Between those two acquisitions — which spans the entire HAL map — map_state is observably Idle. A concurrent unmap reads that Idle and lands in unmap_inner's Idle => Err(NotMapped) arm. The Active arm had the same shape, so there were two observable-Idle windows.

The fix holds a single guard for the function. A concurrent unmap now blocks and then unmaps the Active mapping the call installs — a valid linearization, where NotMapped corresponded to no legal ordering at all.

The old comment warned that holding the lock across the match would deadlock "when we try to re-lock the buffer back to active". That re-lock is what this change removes — the stores reuse the guard already held.

Holding the guard across device::map_buffer also holds it across that function's initialization_status write, so BUFFER_MAP_STATE gains a BUFFER_INITIALIZATION_STATUS edge in lock::rank. The edge is acyclic: initialization_status is a leaf rank, and no path acquires map_state while holding a buffer's init tracker.

The third commit addresses a hazard the guard would otherwise introduce. Device::handle_hal_error calls Device::lose, which invokes the user's device-lost callback inline. On the HAL-map failure path that ran under the new guard, so a callback touching the same buffer (unmap, destroy → unmap, or map_async) would re-lock a non-reentrant mutex on the same thread; it also acquires DEVICE_LOST_CLOSURE, which is not a declared follower of BUFFER_MAP_STATE. map_buffer_deferred_error returns the HAL error untranslated so Buffer::map can release the guard first and translate after — map_state is already Idle there. map_buffer keeps its signature for the two buffer-creation call sites, which hold no such guard.

Testing

Reproduced on unmodified trunk with a stress harness using only the public API on the noop backend — thread A map_async(Read) then poll(Wait), thread B one unmap() per iteration at swept phase offsets:

• trunk: 242 hits / 600k iterations, every one with the impossible-under-atomic-semantics signature (unmap returns NotMapped while the same pending mapAsync resolves Ok), and every one confirmed by an immediate second unmap() succeeding with no intervening map_async — i.e. the state went Idle → Active with no legal writer.
• this PR: 0 hits / 600k.

Under --cfg wgpu_validate_locks: trunk 178 hits / 200k, this PR 0 / 200k with no rank panics — the new edge is exercised on every successful map and the ranked validator accepts the guard discipline.

Those validator runs need #9960 applied on top; without it the mapping path aborts on a pre-existing release-order violation in PendingSubmission before reaching any of this. #9960 is independent of this PR and can land in either order.

cargo xtask test buffer — 330/331 on a discrete and an integrated GPU (real Vulkan and DX12 HAL maps), no hangs. The one failure, wgpu_examples::big_compute_buffers::tests::two_buffers on the integrated GPU, reproduces identically on unmodified trunk. cargo test -p wgpu-core --all-features — 65 pass.

The harness is not included here. It is ~130 lines of public API and could be trimmed into a regression test; happy to add it if wanted.

Note for reviewers: on GLES, map_buffer for MAP_READ does a glGetBufferSubData readback under the global context lock, so on that backend a concurrent same-buffer unmap/map_async can now block for the duration of a GPU sync rather than returning early. It is per-buffer and bounded, and the early return it replaces was a spurious error, but it is a real behavioural change on that backend.

Squash or Rebase?

Three commits, each independently reviewable; ready to rebase. Happy to squash if preferred.

Checklist

• I self-reviewed and fully understand this PR.
• WebGPU implementations built with wgpu may be affected behaviorally.
• Validation and feature gates are in place to confine behavioral changes.
• Tests demonstrate the validation and altered logic works.
• CHANGELOG.md entries for the user-facing effects of this change are present.
• The PR is minimal, and doesn't make sense to land as multiple PRs.
• Commits are logically scoped and individually reviewable.
• The PR description has enough context to understand the motivation and solution implemented.

[ErichDonGubler]

#9960 is reviewed and mostly ready to merge. 🎉