Skip to content

Commit

Permalink
Implement Device validity, and make it invalid when Device is lost.
Browse files Browse the repository at this point in the history
This tracks Device validity explicity, but it could be changed to instead
change Device Storage to Error when the Device is invalid. This also could
go further with standardizing the error reporting for an invalid Device.
  • Loading branch information
bradwerth committed Sep 11, 2023
1 parent e47dc2a commit fff56b6
Show file tree
Hide file tree
Showing 8 changed files with 130 additions and 9 deletions.
3 changes: 3 additions & 0 deletions wgpu-core/src/command/clear.rs
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,9 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
}

let device = &device_guard[cmd_buf.device_id.value];
if !device.is_valid() {
return Err(ClearError::InvalidDevice(cmd_buf.device_id.value.0));
}

clear_texture(
&*texture_guard,
Expand Down
6 changes: 6 additions & 0 deletions wgpu-core/src/command/compute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use crate::{
hal_api::HalApi,
hub::Token,
id,
id::DeviceId,
identity::GlobalIdentityHandlerFactory,
init_tracker::MemoryInitKind,
pipeline,
Expand Down Expand Up @@ -193,6 +194,8 @@ pub enum ComputePassErrorInner {
Encoder(#[from] CommandEncoderError),
#[error("Bind group {0:?} is invalid")]
InvalidBindGroup(id::BindGroupId),
#[error("Device {0:?} is invalid")]
InvalidDevice(DeviceId),
#[error("Bind group index {index} is greater than the device's requested `max_bind_group` limit {max}")]
BindGroupIndexOutOfRange { index: u32, max: u32 },
#[error("Compute pipeline {0:?} is invalid")]
Expand Down Expand Up @@ -390,6 +393,9 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
let raw = cmd_buf.encoder.open();

let device = &device_guard[cmd_buf.device_id.value];
if !device.is_valid() {
return Err(ComputePassErrorInner::InvalidDevice(cmd_buf.device_id.value.0)).map_pass_err(init_scope);
}

#[cfg(feature = "trace")]
if let Some(ref mut list) = cmd_buf.commands {
Expand Down
6 changes: 6 additions & 0 deletions wgpu-core/src/command/render.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ use crate::{
hal_api::HalApi,
hub::Token,
id,
id::DeviceId,
identity::GlobalIdentityHandlerFactory,
init_tracker::{MemoryInitKind, TextureInitRange, TextureInitTrackerAction},
pipeline::{self, PipelineFlags},
Expand Down Expand Up @@ -523,6 +524,8 @@ pub enum RenderPassErrorInner {
ColorAttachment(#[from] ColorAttachmentError),
#[error(transparent)]
Encoder(#[from] CommandEncoderError),
#[error("Device {0:?} is invalid")]
InvalidDevice(DeviceId),
#[error("Attachment texture view {0:?} is invalid")]
InvalidAttachment(id::TextureViewId),
#[error("The format of the depth-stencil attachment ({0:?}) is not a depth-stencil format")]
Expand Down Expand Up @@ -1346,6 +1349,9 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
}

let device = &device_guard[cmd_buf.device_id.value];
if !device.is_valid() {
return Err(RenderPassErrorInner::InvalidDevice(cmd_buf.device_id.value.0)).map_pass_err(init_scope);
}
cmd_buf.encoder.open_pass(base.label);

let (bundle_guard, mut token) = hub.render_bundles.read(&mut token);
Expand Down
16 changes: 15 additions & 1 deletion wgpu-core/src/command/transfer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use crate::{
global::Global,
hal_api::HalApi,
hub::Token,
id::{BufferId, CommandEncoderId, TextureId, Valid},
id::{BufferId, CommandEncoderId, DeviceId, TextureId, Valid},
identity::GlobalIdentityHandlerFactory,
init_tracker::{
has_copy_partial_init_tracker_coverage, MemoryInitKind, TextureInitRange,
Expand Down Expand Up @@ -40,6 +40,8 @@ pub enum CopySide {
#[derive(Clone, Debug, Error)]
#[non_exhaustive]
pub enum TransferError {
#[error("Device {0:?} is invalid")]
InvalidDevice(DeviceId),
#[error("Buffer {0:?} is invalid or destroyed")]
InvalidBuffer(BufferId),
#[error("Texture {0:?} is invalid or destroyed")]
Expand Down Expand Up @@ -569,6 +571,9 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
let (buffer_guard, _) = hub.buffers.read(&mut token);

let device = &device_guard[cmd_buf.device_id.value];
if !device.is_valid() {
return Err(TransferError::InvalidDevice(cmd_buf.device_id.value.0).into());
}

#[cfg(feature = "trace")]
if let Some(ref mut list) = cmd_buf.commands {
Expand Down Expand Up @@ -714,6 +719,9 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
let (texture_guard, _) = hub.textures.read(&mut token);

let device = &device_guard[cmd_buf.device_id.value];
if !device.is_valid() {
return Err(TransferError::InvalidDevice(cmd_buf.device_id.value.0).into());
}

#[cfg(feature = "trace")]
if let Some(ref mut list) = cmd_buf.commands {
Expand Down Expand Up @@ -858,6 +866,9 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
let (texture_guard, _) = hub.textures.read(&mut token);

let device = &device_guard[cmd_buf.device_id.value];
if !device.is_valid() {
return Err(TransferError::InvalidDevice(cmd_buf.device_id.value.0).into());
}

#[cfg(feature = "trace")]
if let Some(ref mut list) = cmd_buf.commands {
Expand Down Expand Up @@ -1020,6 +1031,9 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
let (texture_guard, _) = hub.textures.read(&mut token);

let device = &device_guard[cmd_buf.device_id.value];
if !device.is_valid() {
return Err(TransferError::InvalidDevice(cmd_buf.device_id.value.0).into());
}

#[cfg(feature = "trace")]
if let Some(ref mut list) = cmd_buf.commands {
Expand Down
84 changes: 81 additions & 3 deletions wgpu-core/src/device/global.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,9 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
Ok(device) => device,
Err(_) => break DeviceError::Invalid.into(),
};
if !device.valid {
break DeviceError::Invalid.into();
}

if desc.usage.is_empty() {
// Per spec, `usage` must not be zero.
Expand Down Expand Up @@ -586,6 +589,9 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
Ok(device) => device,
Err(_) => break DeviceError::Invalid.into(),
};
if !device.valid {
break DeviceError::Invalid.into();
}
#[cfg(feature = "trace")]
if let Some(ref trace) = device.trace {
trace
Expand Down Expand Up @@ -641,6 +647,9 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
Ok(device) => device,
Err(_) => break DeviceError::Invalid.into(),
};
if !device.valid {
break DeviceError::Invalid.into();
}

// NB: Any change done through the raw texture handle will not be
// recorded in the replay
Expand Down Expand Up @@ -717,6 +726,9 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
Ok(device) => device,
Err(_) => break DeviceError::Invalid.into(),
};
if !device.valid {
break DeviceError::Invalid.into();
}

// NB: Any change done through the raw buffer handle will not be
// recorded in the replay
Expand Down Expand Up @@ -978,6 +990,10 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
Ok(device) => device,
Err(_) => break DeviceError::Invalid.into(),
};
if !device.valid {
break DeviceError::Invalid.into();
}

#[cfg(feature = "trace")]
if let Some(ref trace) = device.trace {
trace
Expand Down Expand Up @@ -1056,6 +1072,10 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
Ok(device) => device,
Err(_) => break DeviceError::Invalid.into(),
};
if !device.valid {
break DeviceError::Invalid.into();
}

#[cfg(feature = "trace")]
if let Some(ref trace) = device.trace {
trace
Expand Down Expand Up @@ -1170,6 +1190,10 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
Ok(device) => device,
Err(_) => break DeviceError::Invalid.into(),
};
if !device.valid {
break DeviceError::Invalid.into();
}

#[cfg(feature = "trace")]
if let Some(ref trace) = device.trace {
trace
Expand Down Expand Up @@ -1249,6 +1273,10 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
Ok(device) => device,
Err(_) => break DeviceError::Invalid.into(),
};
if !device.valid {
break DeviceError::Invalid.into();
}

#[cfg(feature = "trace")]
if let Some(ref trace) = device.trace {
trace
Expand Down Expand Up @@ -1351,6 +1379,10 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
Ok(device) => device,
Err(_) => break DeviceError::Invalid.into(),
};
if !device.valid {
break DeviceError::Invalid.into();
}

#[cfg(feature = "trace")]
if let Some(ref trace) = device.trace {
let mut trace = trace.lock();
Expand Down Expand Up @@ -1416,6 +1448,10 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
Ok(device) => device,
Err(_) => break DeviceError::Invalid.into(),
};
if !device.valid {
break DeviceError::Invalid.into();
}

#[cfg(feature = "trace")]
if let Some(ref trace) = device.trace {
let mut trace = trace.lock();
Expand Down Expand Up @@ -1486,6 +1522,10 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
Ok(device) => device,
Err(_) => break DeviceError::Invalid,
};
if !device.valid {
break DeviceError::Invalid;
}

let dev_stored = Stored {
value: id::Valid(device_id),
ref_count: device.life_guard.add_ref(),
Expand Down Expand Up @@ -1579,6 +1619,10 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
Ok(device) => device,
Err(_) => break command::RenderBundleError::INVALID_DEVICE,
};
if !device.valid {
break command::RenderBundleError::INVALID_DEVICE;
}

#[cfg(feature = "trace")]
if let Some(ref trace) = device.trace {
trace.lock().add(trace::Action::CreateRenderBundle {
Expand Down Expand Up @@ -1661,6 +1705,10 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
Ok(device) => device,
Err(_) => break DeviceError::Invalid.into(),
};
if !device.valid {
break DeviceError::Invalid.into();
}

#[cfg(feature = "trace")]
if let Some(ref trace) = device.trace {
trace.lock().add(trace::Action::CreateQuerySet {
Expand Down Expand Up @@ -1750,6 +1798,10 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
Ok(device) => device,
Err(_) => break DeviceError::Invalid.into(),
};
if !device.valid {
break DeviceError::Invalid.into();
}

let adapter = &adapter_guard[device.adapter_id.value];
#[cfg(feature = "trace")]
if let Some(ref trace) = device.trace {
Expand Down Expand Up @@ -1893,6 +1945,10 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
Ok(device) => device,
Err(_) => break DeviceError::Invalid.into(),
};
if !device.valid {
break DeviceError::Invalid.into();
}

#[cfg(feature = "trace")]
if let Some(ref trace) = device.trace {
trace.lock().add(trace::Action::CreateComputePipeline {
Expand Down Expand Up @@ -2141,6 +2197,10 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
Ok(device) => device,
Err(_) => break DeviceError::Invalid.into(),
};
if !device.valid {
break DeviceError::Invalid.into();
}

#[cfg(feature = "trace")]
if let Some(ref trace) = device.trace {
trace
Expand Down Expand Up @@ -2423,6 +2483,22 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
}
}

pub fn device_lose<A: HalApi>(&self, device_id: DeviceId) {
let hub = A::hub(self);
let mut token = Token::root();

let (mut device_guard, _) = hub.devices.write(&mut token);
if let Ok(device) = device_guard.get_mut(device_id) {
// Mark the device explicitly as invalid. This is checked in various
// places to prevent new work from being submitted, allowing a later
// call to `poll_devices` to drop this device when the work queues
// are cleared.
device.valid = false;
}

// TODO(BJW): inform the GPUDevice that it is lost, and provide a reason.
}

/// Exit the unreferenced, inactive device `device_id`.
fn exit_device<A: HalApi>(&self, device_id: DeviceId) {
let hub = A::hub(self);
Expand Down Expand Up @@ -2507,6 +2583,11 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
}
};

let device = &device_guard[buffer.device_id.value];
if !device.valid {
return Err((op, BufferAccessError::Invalid));
}

if let Err(e) = check_buffer_usage(buffer.usage, pub_usage) {
return Err((op, e.into()));
}
Expand Down Expand Up @@ -2547,8 +2628,6 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
};
log::debug!("Buffer {:?} map state -> Waiting", buffer_id);

let device = &device_guard[buffer.device_id.value];

let ret = (buffer.device_id.value, buffer.life_guard.add_ref());

let mut trackers = device.trackers.lock();
Expand All @@ -2561,7 +2640,6 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
};

let device = &device_guard[device_id];

device
.lock_life(&mut token)
.map(id::Valid(buffer_id), ref_count);
Expand Down
8 changes: 3 additions & 5 deletions wgpu-core/src/device/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -293,11 +293,9 @@ pub struct InvalidDevice;

#[derive(Clone, Debug, Error)]
pub enum DeviceError {
#[error("Parent device is invalid")]
#[error("Parent device is invalid or lost.")]
Invalid,
#[error("Parent device is lost")]
Lost,
#[error("Not enough memory left")]
#[error("Not enough memory left.")]
OutOfMemory,
#[error("Creation of a resource failed for a reason other than running out of memory.")]
ResourceCreationFailed,
Expand All @@ -306,7 +304,7 @@ pub enum DeviceError {
impl From<hal::DeviceError> for DeviceError {
fn from(error: hal::DeviceError) -> Self {
match error {
hal::DeviceError::Lost => DeviceError::Lost,
hal::DeviceError::Lost => DeviceError::Invalid,
hal::DeviceError::OutOfMemory => DeviceError::OutOfMemory,
hal::DeviceError::ResourceCreationFailed => DeviceError::ResourceCreationFailed,
}
Expand Down
7 changes: 7 additions & 0 deletions wgpu-core/src/device/resource.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ pub struct Device<A: HalApi> {
pub(crate) active_submission_index: SubmissionIndex,
pub(super) fence: A::Fence,

pub(super) valid: bool,

/// All live resources allocated with this [`Device`].
///
/// Has to be locked temporarily only (locked last)
Expand Down Expand Up @@ -189,6 +191,7 @@ impl<A: HalApi> Device<A> {
command_allocator: Mutex::new(com_alloc),
active_submission_index: 0,
fence,
valid: true,
trackers: Mutex::new(Tracker::new()),
life_tracker: Mutex::new(life::LifetimeTracker::new()),
temp_suspected: life::SuspectedResources::default(),
Expand All @@ -214,6 +217,10 @@ impl<A: HalApi> Device<A> {
})
}

pub fn is_valid(&self) -> bool {
self.valid
}

pub(super) fn lock_life<'this, 'token: 'this>(
&'this self,
//TODO: fix this - the token has to be borrowed for the lock
Expand Down
Loading

0 comments on commit fff56b6

Please sign in to comment.