Make DeviceLostClosure.from_c consume the closure before dropping it. (#5032)

This clarifies that the Rust and C-style callbacks/closures need to be
consumed (not called) before they are dropped. It also makes the from_c
function consume the param closure so that it can be dropped without
panicking.

It also relaxes the restriction that the callback/closure can only be
called once.

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

wgpu-core/src/device/mod.rs

@@ -212,13 +212,13 @@ pub type DeviceLostCallback = Box<dyn Fn(DeviceLostReason, String) + 'static>;
 
 pub struct DeviceLostClosureRust {
     pub callback: DeviceLostCallback,
-    called: bool,
+    consumed: bool,
 }
 
 impl Drop for DeviceLostClosureRust {
     fn drop(&mut self) {
-        if !self.called {
+        if !self.consumed {
-            panic!("DeviceLostClosureRust must be called before it is dropped.");
+            panic!("DeviceLostClosureRust must be consumed before it is dropped.");
         }
     }
 }
@@ -227,7 +227,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(send_sync)]
@@ -235,8 +235,8 @@ unsafe impl Send for DeviceLostClosureC {}
 
 impl Drop for DeviceLostClosureC {
     fn drop(&mut self) {
-        if !self.called {
+        if !self.consumed {
-            panic!("DeviceLostClosureC must be called before it is dropped.");
+            panic!("DeviceLostClosureC must be consumed before it is dropped.");
         }
     }
 }
@@ -262,7 +262,7 @@ impl DeviceLostClosure {
     pub fn from_rust(callback: DeviceLostCallback) -> Self {
         let inner = DeviceLostClosureRust {
             callback,
-            called: false,
+            consumed: false,
         };
         Self {
             inner: DeviceLostClosureInner::Rust { inner },
@@ -276,14 +276,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 },
         }
@@ -292,19 +296,13 @@ impl DeviceLostClosure {
     pub(crate) fn call(self, reason: DeviceLostReason, message: String) {
         match self.inner {
             DeviceLostClosureInner::Rust { mut inner } => {
-                if inner.called {
+                inner.consumed = true;
-                    panic!("DeviceLostClosureRust must only be called once.");
-                }
-                inner.called = 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 {
+                inner.consumed = true;
-                    panic!("DeviceLostClosureC must only be called once.");
-                }
-                inner.called = true;
 
                 // Ensure message is structured as a null-terminated C string. It only
                 // needs to live as long as the callback invocation.