Skip to content

Commit d7a3308

Browse files
committed
[core] Doc fixes related to mapping callback management.
Add a few comments to tricky corners involved in ensuring that mapping callbacks are always invoked, never dropped.
1 parent fa37706 commit d7a3308

3 files changed

Lines changed: 28 additions & 2 deletions

File tree

wgpu-core/src/device/life.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -384,6 +384,11 @@ impl LifetimeTracker {
384384
for buffer in self.ready_to_map.drain(..) {
385385
match buffer.map(snatch_guard) {
386386
Some(cb) => pending_callbacks.push(cb),
387+
// `None` means the mapping was cancelled between when this
388+
// buffer was added to `ready_to_map` and now — typically
389+
// because `unmap` was called on it. `unmap_inner` already fired
390+
// the callback with `MapAborted` in that case, so there is
391+
// nothing left to do here.
387392
None => continue,
388393
}
389394
}

wgpu-core/src/device/queue.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -281,6 +281,12 @@ impl Drop for Queue {
281281
self.maintain(last_successful_submission_index, &snatch_guard);
282282
drop(snatch_guard);
283283

284+
// `wait_for_idle` above ensures all in-flight submissions have
285+
// completed, so `maintain` should have drained every active
286+
// submission. A false result here indicates that something went
287+
// wrong — most likely the device was lost and `wait_for_idle`
288+
// did not surface that as an error, leaving submissions
289+
// unprocessed and their `mapped` buffer callbacks unfired.
284290
assert!(queue_empty);
285291

286292
let closures = crate::device::UserClosures {

wgpu-core/src/resource.rs

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -438,7 +438,16 @@ pub type BufferAccessResult = Result<(), BufferAccessError>;
438438
pub(crate) struct BufferPendingMapping {
439439
pub(crate) range: Range<wgt::BufferAddress>,
440440
pub(crate) op: BufferMapOperation,
441-
// hold the parent alive while the mapping is active
441+
442+
/// A strong reference to the parent buffer, to hold it alive
443+
/// while the mapping is pending.
444+
///
445+
/// This creates a temporary reference cycle: [`Buffer::map_state`] owns
446+
/// this `BufferPendingMapping`, which in turn holds an `Arc<Buffer>` back
447+
/// to the same buffer. The cycle is intentional — it keeps the buffer alive
448+
/// while it sits in `LifetimeTracker::ready_to_map` with no other owner —
449+
/// and is broken by `Buffer::map` when it moves the `BufferPendingMapping`
450+
/// out of [`Buffer::map_state`].
442451
pub(crate) _parent_buffer: Arc<Buffer>,
443452
}
444453

@@ -716,6 +725,12 @@ impl Buffer {
716725
}
717726

718727
if let Some(queue) = device.get_queue().as_ref() {
728+
// Flush pending writes to this buffer before scheduling the map.
729+
//
730+
// Such writes get added to `queue.life_tracker.lock().active`, so
731+
// that `lock_life().map` below will find the buffer in that
732+
// submission and wait for it, rather than placing it in
733+
// `ready_to_map` prematurely.
719734
match queue.flush_writes_for_buffer(self, snatch_guard) {
720735
Err(err) => {
721736
let state = mem::replace(&mut *self.map_state.lock(), BufferMapState::Idle);
@@ -725,7 +740,7 @@ impl Buffer {
725740
return Err((op, err));
726741
}
727742
Ok(()) => {
728-
// Schedule the buffer map in the lifetime tracker.
743+
// Schedule the buffer map in the lifetime tracker.
729744
//
730745
// This call searches for use of the buffer by pending submissions.
731746
// If we just flushed pending writes, that search is redundant; we

0 commit comments

Comments
 (0)