[core] Enforce a deadlock-free locking order for Mutexes.

If `debug_assertions` or the `"validate-locks"` feature are enabled,
change `wgpu-core` to use a wrapper around `parking_lot::Mutex` that
checks for potential deadlocks.

At the moment, `wgpu-core` does contain deadlocks, so the ranking in
the `lock::rank` module is incomplete, in the interests of keeping it
acyclic. gfx-rs#5572 tracks the work needed to complete the ranking.

wgpu-core/src/command/allocator.rs

@@ -2,7 +2,7 @@ use crate::hal_api::HalApi;
 use crate::resource_log;
 use hal::Device as _;
 
-use parking_lot::Mutex;
+use crate::lock::{rank, Mutex};
 
 /// A pool of free [`wgpu_hal::CommandEncoder`]s, owned by a `Device`.
 ///
@@ -21,7 +21,7 @@ pub(crate) struct CommandAllocator<A: HalApi> {
 impl<A: HalApi> CommandAllocator<A> {
  pub(crate) fn new() -> Self {
  Self {
- free_encoders: Mutex::new(Vec::new()),
+ free_encoders: Mutex::new(rank::COMMAND_ALLOCATOR_FREE_ENCODERS, Vec::new()),
  }
  }
 

wgpu-core/src/command/mod.rs

@@ -23,6 +23,7 @@ use crate::device::{Device, DeviceError};
 use crate::error::{ErrorFormatter, PrettyError};
 use crate::hub::Hub;
 use crate::id::CommandBufferId;
+use crate::lock::{rank, Mutex};
 use crate::snatch::SnatchGuard;
 
 use crate::init_tracker::BufferInitTrackerAction;
@@ -31,7 +32,6 @@ use crate::track::{Tracker, UsageScope};
 use crate::{api_log, global::Global, hal_api::HalApi, id, resource_log, Label};
 
 use hal::CommandEncoder as _;
-use parking_lot::Mutex;
 use thiserror::Error;
 
 #[cfg(feature = "trace")]
@@ -338,25 +338,28 @@ impl<A: HalApi> CommandBuffer<A> {
  .as_str(),
  None,
  ),
- data: Mutex::new(Some(CommandBufferMutable {
- encoder: CommandEncoder {
- raw: encoder,
- is_open: false,
- list: Vec::new(),
- label,
- },
- status: CommandEncoderStatus::Recording,
- trackers: Tracker::new(),
- buffer_memory_init_actions: Default::default(),
- texture_memory_actions: Default::default(),
- pending_query_resets: QueryResetMap::new(),
- #[cfg(feature = "trace")]
- commands: if enable_tracing {
- Some(Vec::new())
- } else {
- None
- },
- })),
+ data: Mutex::new(
+ rank::COMMAND_BUFFER_DATA,
+ Some(CommandBufferMutable {
+ encoder: CommandEncoder {
+ raw: encoder,
+ is_open: false,
+ list: Vec::new(),
+ label,
+ },
+ status: CommandEncoderStatus::Recording,
+ trackers: Tracker::new(),
+ buffer_memory_init_actions: Default::default(),
+ texture_memory_actions: Default::default(),
+ pending_query_resets: QueryResetMap::new(),
+ #[cfg(feature = "trace")]
+ commands: if enable_tracing {
+ Some(Vec::new())
+ } else {
+ None
+ },
+ }),
+ ),
  }
  }
 

wgpu-core/src/device/life.rs

@@ -7,6 +7,7 @@ use crate::{
  },
  hal_api::HalApi,
  id,
+ lock::Mutex,
  pipeline::{ComputePipeline, RenderPipeline},
  resource::{
  self, Buffer, DestroyedBuffer, DestroyedTexture, QuerySet, Resource, Sampler,
@@ -18,7 +19,6 @@ use crate::{
 };
 use smallvec::SmallVec;
 
-use parking_lot::Mutex;
 use std::sync::Arc;
 use thiserror::Error;
 

wgpu-core/src/device/queue.rs

@@ -14,6 +14,7 @@ use crate::{
  hal_label,
  id::{self, DeviceId, QueueId},
  init_tracker::{has_copy_partial_init_tracker_coverage, TextureInitRange},
+ lock::{rank, Mutex},
  resource::{
  Buffer, BufferAccessError, BufferMapState, DestroyedBuffer, DestroyedTexture, Resource,
  ResourceInfo, ResourceType, StagingBuffer, Texture, TextureInner,
@@ -22,7 +23,6 @@ use crate::{
 };
 
 use hal::{CommandEncoder as _, Device as _, Queue as _};
-use parking_lot::Mutex;
 use smallvec::SmallVec;
 
 use std::{
@@ -317,7 +317,7 @@ fn prepare_staging_buffer<A: HalApi>(
  let mapping = unsafe { device.raw().map_buffer(&buffer, 0..size) }?;
 
  let staging_buffer = StagingBuffer {
- raw: Mutex::new(Some(buffer)),
+ raw: Mutex::new(rank::STAGING_BUFFER_RAW, Some(buffer)),
  device: device.clone(),
  size,
  info: ResourceInfo::new(

wgpu-core/src/device/resource.rs

@@ -18,6 +18,7 @@ use crate::{
  TextureInitTracker, TextureInitTrackerAction,
  },
  instance::Adapter,
+ lock::{rank, Mutex, MutexGuard},
  pipeline,
  pool::ResourcePool,
  registry::Registry,
@@ -41,7 +42,7 @@ use crate::{
 use arrayvec::ArrayVec;
 use hal::{CommandEncoder as _, Device as _};
 use once_cell::sync::OnceCell;
-use parking_lot::{Mutex, MutexGuard, RwLock};
+use parking_lot::RwLock;
 
 use smallvec::SmallVec;
 use thiserror::Error;
@@ -274,33 +275,39 @@ impl<A: HalApi> Device<A> {
  fence: RwLock::new(Some(fence)),
  snatchable_lock: unsafe { SnatchLock::new() },
  valid: AtomicBool::new(true),
- trackers: Mutex::new(Tracker::new()),
+ trackers: Mutex::new(rank::DEVICE_TRACKERS, Tracker::new()),
  tracker_indices: TrackerIndexAllocators::new(),
- life_tracker: Mutex::new(life::LifetimeTracker::new()),
- temp_suspected: Mutex::new(Some(life::ResourceMaps::new())),
+ life_tracker: Mutex::new(rank::DEVICE_LIFE_TRACKER, life::LifetimeTracker::new()),
+ temp_suspected: Mutex::new(
+ rank::DEVICE_TEMP_SUSPECTED,
+ Some(life::ResourceMaps::new()),
+ ),
  bgl_pool: ResourcePool::new(),
  #[cfg(feature = "trace")]
- trace: Mutex::new(trace_path.and_then(|path| match trace::Trace::new(path) {
- Ok(mut trace) => {
- trace.add(trace::Action::Init {
- desc: desc.clone(),
- backend: A::VARIANT,
- });
- Some(trace)
- }
- Err(e) => {
- log::error!("Unable to start a trace in '{path:?}': {e}");
- None
- }
- })),
+ trace: Mutex::new(
+ rank::DEVICE_TRACE,
+ trace_path.and_then(|path| match trace::Trace::new(path) {
+ Ok(mut trace) => {
+ trace.add(trace::Action::Init {
+ desc: desc.clone(),
+ backend: A::VARIANT,
+ });
+ Some(trace)
+ }
+ Err(e) => {
+ log::error!("Unable to start a trace in '{path:?}': {e}");
+ None
+ }
+ }),
+ ),
  alignments,
  limits: desc.required_limits.clone(),
  features: desc.required_features,
  downlevel,
  instance_flags,
- pending_writes: Mutex::new(Some(pending_writes)),
- deferred_destroy: Mutex::new(Vec::new()),
- usage_scopes: Default::default(),
+ pending_writes: Mutex::new(rank::DEVICE_PENDING_WRITES, Some(pending_writes)),
+ deferred_destroy: Mutex::new(rank::DEVICE_DEFERRED_DESTROY, Vec::new()),
+ usage_scopes: Mutex::new(rank::DEVICE_USAGE_SCOPES, Default::default()),
  })
  }
 
@@ -650,13 +657,13 @@ impl<A: HalApi> Device<A> {
  usage: desc.usage,
  size: desc.size,
  initialization_status: RwLock::new(BufferInitTracker::new(aligned_size)),
- sync_mapped_writes: Mutex::new(None),
- map_state: Mutex::new(resource::BufferMapState::Idle),
+ sync_mapped_writes: Mutex::new(rank::BUFFER_SYNC_MAPPED_WRITES, None),
+ map_state: Mutex::new(rank::BUFFER_MAP_STATE, resource::BufferMapState::Idle),
  info: ResourceInfo::new(
  desc.label.borrow_or_default(),
  Some(self.tracker_indices.buffers.clone()),
  ),
- bind_groups: Mutex::new(Vec::new()),
+ bind_groups: Mutex::new(rank::BUFFER_BIND_GROUPS, Vec::new()),
  })
  }
 
@@ -689,8 +696,8 @@ impl<A: HalApi> Device<A> {
  Some(self.tracker_indices.textures.clone()),
  ),
  clear_mode: RwLock::new(clear_mode),
- views: Mutex::new(Vec::new()),
- bind_groups: Mutex::new(Vec::new()),
+ views: Mutex::new(rank::TEXTURE_VIEWS, Vec::new()),
+ bind_groups: Mutex::new(rank::TEXTURE_BIND_GROUPS, Vec::new()),
  }
  }
 
@@ -707,13 +714,13 @@ impl<A: HalApi> Device<A> {
  usage: desc.usage,
  size: desc.size,
  initialization_status: RwLock::new(BufferInitTracker::new(0)),
- sync_mapped_writes: Mutex::new(None),
- map_state: Mutex::new(resource::BufferMapState::Idle),
+ sync_mapped_writes: Mutex::new(rank::BUFFER_SYNC_MAPPED_WRITES, None),
+ map_state: Mutex::new(rank::BUFFER_MAP_STATE, resource::BufferMapState::Idle),
  info: ResourceInfo::new(
  desc.label.borrow_or_default(),
  Some(self.tracker_indices.buffers.clone()),
  ),
- bind_groups: Mutex::new(Vec::new()),
+ bind_groups: Mutex::new(rank::BUFFER_BIND_GROUPS, Vec::new()),
  }
  }
 

wgpu-core/src/identity.rs

@@ -1,8 +1,8 @@
-use parking_lot::Mutex;
 use wgt::Backend;
 
 use crate::{
  id::{Id, Marker},
+ lock::{rank, Mutex},
  Epoch, Index,
 };
 use std::{fmt::Debug, marker::PhantomData};
@@ -117,12 +117,15 @@ impl<T: Marker> IdentityManager<T> {
 impl<T: Marker> IdentityManager<T> {
  pub fn new() -> Self {
  Self {
- values: Mutex::new(IdentityValues {
- free: Vec::new(),
- next_index: 0,
- count: 0,
- id_source: IdSource::None,
- }),
+ values: Mutex::new(
+ rank::IDENTITY_MANAGER_VALUES,
+ IdentityValues {
+ free: Vec::new(),
+ next_index: 0,
+ count: 0,
+ id_source: IdSource::None,
+ },
+ ),
  _phantom: PhantomData,
  }
  }

wgpu-core/src/instance.rs

@@ -6,13 +6,14 @@ use crate::{
  device::{queue::Queue, resource::Device, DeviceDescriptor},
  global::Global,
  hal_api::HalApi,
- id::{markers, AdapterId, DeviceId, Id, Marker, QueueId, SurfaceId},
+ id::markers,
+ id::{AdapterId, DeviceId, Id, Marker, QueueId, SurfaceId},
+ lock::{rank, Mutex},
  present::Presentation,
  resource::{Resource, ResourceInfo, ResourceType},
  resource_log, LabelHelpers, DOWNLEVEL_WARNING_MESSAGE,
 };
 
-use parking_lot::Mutex;
 use wgt::{Backend, Backends, PowerPreference};
 
 use hal::{Adapter as _, Instance as _, OpenDevice};
@@ -530,7 +531,7 @@ impl Global {
  let mut any_created = false;
 
  let surface = Surface {
- presentation: Mutex::new(None),
+ presentation: Mutex::new(rank::SURFACE_PRESENTATION, None),
  info: ResourceInfo::new("<Surface>", None),
 
  #[cfg(vulkan)]
@@ -594,7 +595,7 @@ impl Global {
  profiling::scope!("Instance::create_surface_metal");
 
  let surface = Surface {
- presentation: Mutex::new(None),
+ presentation: Mutex::new(rank::SURFACE_PRESENTATION, None),
  info: ResourceInfo::new("<Surface>", None),
  metal: Some(self.instance.metal.as_ref().map_or(
  Err(CreateSurfaceError::BackendNotEnabled(Backend::Metal)),
@@ -623,7 +624,7 @@ impl Global {
  create_surface_func: impl FnOnce(&HalInstance<hal::api::Dx12>) -> HalSurface<hal::api::Dx12>,
  ) -> Result<SurfaceId, CreateSurfaceError> {
  let surface = Surface {
- presentation: Mutex::new(None),
+ presentation: Mutex::new(rank::SURFACE_PRESENTATION, None),
  info: ResourceInfo::new("<Surface>", None),
  dx12: Some(create_surface_func(
  self.instance

wgpu-core/src/lib.rs

@@ -63,6 +63,7 @@ pub mod id;
 pub mod identity;
 mod init_tracker;
 pub mod instance;
+mod lock;
 pub mod pipeline;
 mod pool;
 pub mod present;

wgpu-core/src/lock/mod.rs

@@ -0,0 +1,41 @@
+//! Instrumented lock types.
+//!
+//! This module defines a set of instrumented wrappers for the lock
+//! types used in `wgpu-core` ([`Mutex`], [`RwLock`], and
+//! [`SnatchLock`]) that help us understand and validate `wgpu-core`
+//! synchronization.
+//!
+//! - The [`ranked`] module defines lock types that perform run-time
+//! checks to ensure that each thread acquires locks only in a
+//! specific order, to prevent deadlocks.
+//!
+//! - The [`vanilla`] module defines lock types that are
+//! uninstrumented, no-overhead wrappers around the standard lock
+//! types.
+//!
+//! (We plan to add more wrappers in the future.)
+//!
+//! If the `wgpu_validate_locks` config is set (for example, with
+//! `RUSTFLAGS='--cfg wgpu_validate_locks'`), `wgpu-core` uses the
+//! [`ranked`] module's locks. We hope to make this the default for
+//! debug builds soon.
+//!
+//! Otherwise, `wgpu-core` uses the [`vanilla`] module's locks.
+//!
+//! [`Mutex`]: parking_lot::Mutex
+//! [`RwLock`]: parking_lot::RwLock
+//! [`SnatchLock`]: crate::snatch::SnatchLock
+
+pub mod rank;
+
+#[cfg_attr(not(wgpu_validate_locks), allow(dead_code))]
+mod ranked;
+
+#[cfg_attr(wgpu_validate_locks, allow(dead_code))]
+mod vanilla;
+
+#[cfg(wgpu_validate_locks)]
+pub use ranked::{Mutex, MutexGuard};
+
+#[cfg(not(wgpu_validate_locks))]
+pub use vanilla::{Mutex, MutexGuard};