diff --git a/CHANGELOG.md b/CHANGELOG.md index 644fa125d5..4483dd1e83 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -189,6 +189,7 @@ Passing an owned value `window` to `Surface` will return a `Surface<'static>`. S - `BufferMappedRange` trait is now `WasmNotSendSync`, i.e. it is `Send`/`Sync` if not on wasm or `fragile-send-sync-non-atomic-wasm` is enabled. By @wumpf in [#4818](https://github.com/gfx-rs/wgpu/pull/4818) - Align `wgpu_types::CompositeAlphaMode` serde serialization to spec. By @littledivy in [#4940](https://github.com/gfx-rs/wgpu/pull/4940) - Fix error message of `ConfigureSurfaceError::TooLarge`. By @Dinnerbone in [#4960](https://github.com/gfx-rs/wgpu/pull/4960) +- Fix dropping of `DeviceLostCallbackC` params. By @bradwerth in [#5032](https://github.com/gfx-rs/wgpu/pull/5032) - Fixed a number of panics. by @nical in [#4999](https://github.com/gfx-rs/wgpu/pull/4999), [#5014](https://github.com/gfx-rs/wgpu/pull/5014), [#5024](https://github.com/gfx-rs/wgpu/pull/5024), [#5025](https://github.com/gfx-rs/wgpu/pull/5025), [#5026](https://github.com/gfx-rs/wgpu/pull/5026), [#5027](https://github.com/gfx-rs/wgpu/pull/5027), [#5028](https://github.com/gfx-rs/wgpu/pull/5028) and [#5042](https://github.com/gfx-rs/wgpu/pull/5042). #### DX12 diff --git a/wgpu-core/src/device/mod.rs b/wgpu-core/src/device/mod.rs index bb0afedafc..fdddaa0935 100644 --- a/wgpu-core/src/device/mod.rs +++ b/wgpu-core/src/device/mod.rs @@ -224,13 +224,13 @@ pub type DeviceLostCallback = Box; pub struct DeviceLostClosureRust { pub callback: DeviceLostCallback, - called: bool, + consumed: bool, } impl Drop for DeviceLostClosureRust { fn drop(&mut self) { - if !self.called { - panic!("DeviceLostClosureRust must be called before it is dropped."); + if !self.consumed { + panic!("DeviceLostClosureRust must be consumed before it is dropped."); } } } @@ -239,7 +239,7 @@ impl Drop for DeviceLostClosureRust { pub struct DeviceLostClosureC { pub callback: unsafe extern "C" fn(user_data: *mut u8, reason: u8, message: *const c_char), pub user_data: *mut u8, - called: bool, + consumed: bool, } #[cfg(any( @@ -253,8 +253,8 @@ unsafe impl Send for DeviceLostClosureC {} impl Drop for DeviceLostClosureC { fn drop(&mut self) { - if !self.called { - panic!("DeviceLostClosureC must be called before it is dropped."); + if !self.consumed { + panic!("DeviceLostClosureC must be consumed before it is dropped."); } } } @@ -280,7 +280,7 @@ impl DeviceLostClosure { pub fn from_rust(callback: DeviceLostCallback) -> Self { let inner = DeviceLostClosureRust { callback, - called: false, + consumed: false, }; Self { inner: DeviceLostClosureInner::Rust { inner }, @@ -294,14 +294,18 @@ impl DeviceLostClosure { /// /// - Both pointers must point to `'static` data, as the callback may happen at /// an unspecified time. - pub unsafe fn from_c(closure: DeviceLostClosureC) -> Self { + pub unsafe fn from_c(mut closure: DeviceLostClosureC) -> Self { // Build an inner with the values from closure, ensuring that - // inner.called is false. + // inner.consumed is false. let inner = DeviceLostClosureC { callback: closure.callback, user_data: closure.user_data, - called: false, + consumed: false, }; + + // Mark the original closure as consumed, so we can safely drop it. + closure.consumed = true; + Self { inner: DeviceLostClosureInner::C { inner }, } @@ -310,19 +314,13 @@ impl DeviceLostClosure { pub(crate) fn call(self, reason: DeviceLostReason, message: String) { match self.inner { DeviceLostClosureInner::Rust { mut inner } => { - if inner.called { - panic!("DeviceLostClosureRust must only be called once."); - } - inner.called = true; + inner.consumed = true; (inner.callback)(reason, message) } // SAFETY: the contract of the call to from_c says that this unsafe is sound. DeviceLostClosureInner::C { mut inner } => unsafe { - if inner.called { - panic!("DeviceLostClosureC must only be called once."); - } - inner.called = true; + inner.consumed = true; // Ensure message is structured as a null-terminated C string. It only // needs to live as long as the callback invocation.