diff --git a/CHANGELOG.md b/CHANGELOG.md index 839ae0572a..99bbcbc48b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -102,6 +102,7 @@ Bottom level categories: - Fix `panic!` when dropping `Instance` without `InstanceFlags::VALIDATION`. By @hakolao in [#5134](https://github.com/gfx-rs/wgpu/pull/5134) - Fix `serde` feature not compiling for `wgpu-types`. By @KirmesBude in [#5149](https://github.com/gfx-rs/wgpu/pull/5149) - Fix the validation of vertex and index ranges. By @nical in [#5144](https://github.com/gfx-rs/wgpu/pull/5144) and [#5156](https://github.com/gfx-rs/wgpu/pull/5156) +- Device lost callbacks are invoked when replaced and when global is dropped. By @bradwerth in [#5168](https://github.com/gfx-rs/wgpu/pull/5168) #### WGL diff --git a/tests/tests/device.rs b/tests/tests/device.rs index 56f5251b92..7743be7242 100644 --- a/tests/tests/device.rs +++ b/tests/tests/device.rs @@ -549,3 +549,69 @@ static DEVICE_DROP_THEN_LOST: GpuTestConfiguration = GpuTestConfiguration::new() "Device lost callback should have been called." ); }); + +#[gpu_test] +static DEVICE_LOST_REPLACED_CALLBACK: GpuTestConfiguration = GpuTestConfiguration::new() + .parameters(TestParameters::default()) + .run_sync(|ctx| { + // This test checks that a device_lost_callback is called when it is + // replaced by another callback. + let was_called = std::sync::Arc::::new(false.into()); + + // Set a LoseDeviceCallback on the device. + let was_called_clone = was_called.clone(); + let callback = Box::new(move |reason, _m| { + was_called_clone.store(true, std::sync::atomic::Ordering::SeqCst); + assert!( + matches!(reason, wgt::DeviceLostReason::ReplacedCallback), + "Device lost info reason should match DeviceLostReason::ReplacedCallback." + ); + }); + ctx.device.set_device_lost_callback(callback); + + // Replace the callback. + let replacement_callback = Box::new(move |_r, _m| {}); + ctx.device.set_device_lost_callback(replacement_callback); + + assert!( + was_called.load(std::sync::atomic::Ordering::SeqCst), + "Device lost callback should have been called." + ); + }); + +#[gpu_test] +static DROPPED_GLOBAL_THEN_DEVICE_LOST: GpuTestConfiguration = GpuTestConfiguration::new() + .parameters(TestParameters::default().skip(FailureCase::always())) + .run_sync(|ctx| { + // What we want to do is to drop the Global, forcing a code path that + // eventually calls Device.prepare_to_die, without having first dropped + // the device. This models what might happen in a user agent that kills + // wgpu without providing a more orderly shutdown. In such a case, the + // device lost callback should be invoked with the message "Device is + // dying." + let was_called = std::sync::Arc::::new(false.into()); + + // Set a LoseDeviceCallback on the device. + let was_called_clone = was_called.clone(); + let callback = Box::new(move |reason, message| { + was_called_clone.store(true, std::sync::atomic::Ordering::SeqCst); + assert!( + matches!(reason, wgt::DeviceLostReason::Dropped), + "Device lost info reason should match DeviceLostReason::Dropped." + ); + assert!( + message == "Device is dying.", + "Device lost info message is \"{}\" and it should be \"Device is dying.\".", + message + ); + }); + ctx.device.set_device_lost_callback(callback); + + // TODO: Drop the Global, somehow. + + // Confirm that the callback was invoked. + assert!( + was_called.load(std::sync::atomic::Ordering::SeqCst), + "Device lost callback should have been called." + ); + }); diff --git a/wgpu-core/src/device/global.rs b/wgpu-core/src/device/global.rs index daa42fddef..a6fe43835c 100644 --- a/wgpu-core/src/device/global.rs +++ b/wgpu-core/src/device/global.rs @@ -2266,8 +2266,8 @@ impl Global { } } - // This closure will be called exactly once during "lose the device" - // or when the device is dropped, if it was never lost. + // This closure will be called exactly once during "lose the device", + // or when it is replaced. pub fn device_set_device_lost_closure( &self, device_id: DeviceId, @@ -2277,6 +2277,12 @@ impl Global { if let Ok(device) = hub.devices.get(device_id) { let mut life_tracker = device.lock_life(); + if let Some(existing_closure) = life_tracker.device_lost_closure.take() { + // It's important to not hold the lock while calling the closure. + drop(life_tracker); + existing_closure.call(DeviceLostReason::ReplacedCallback, "".to_string()); + life_tracker = device.lock_life(); + } life_tracker.device_lost_closure = Some(device_lost_closure); } } diff --git a/wgpu-core/src/device/resource.rs b/wgpu-core/src/device/resource.rs index b9b942449e..78f7ddee0a 100644 --- a/wgpu-core/src/device/resource.rs +++ b/wgpu-core/src/device/resource.rs @@ -3433,6 +3433,11 @@ impl Device { current_index, self.command_allocator.lock().as_mut().unwrap(), ); + if let Some(device_lost_closure) = life_tracker.device_lost_closure.take() { + // It's important to not hold the lock while calling the closure. + drop(life_tracker); + device_lost_closure.call(DeviceLostReason::Dropped, "Device is dying.".to_string()); + } #[cfg(feature = "trace")] { *self.trace.lock() = None; diff --git a/wgpu-types/src/lib.rs b/wgpu-types/src/lib.rs index bd3845b75c..34a7f130ff 100644 --- a/wgpu-types/src/lib.rs +++ b/wgpu-types/src/lib.rs @@ -7062,4 +7062,11 @@ pub enum DeviceLostReason { /// we invoke the callback on drop to help with managing memory owned by /// the callback. Dropped = 2, + /// After replacing the device_lost_callback + /// + /// WebGPU does not have a concept of a device lost callback, but wgpu + /// does. wgpu guarantees that any supplied callback will be invoked + /// exactly once before it is dropped, which helps with managing the + /// memory owned by the callback. + ReplacedCallback = 3, }