Merge #258

258: Run-time lock protection against double root r=grovesNL a=kvark

Fixes https://github.com/gfx-rs/wgpu-rs/issues/42
cc @paulkernfeld 

We didn't handle a case where the root locking token would get dropped (while some children are borrowed), and a new one is created. This was the case in `wgpu_device_poll`, which ended up trying to unmap the buffers.

This PR brings a relatively simple run-time check for this. It could *probably* be done at the type level, but I'm going to leave it for any follow ups (help is welcome!), because:
  1. we'll still have a run-time check for the simple case where 2 or more root tokens are created
  2. I spent 20 minutes trying and wasn't able to get this going

Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>

wgpu-native/src/device.rs

@@ -2000,8 +2000,10 @@ pub extern "C" fn wgpu_device_create_swap_chain(
 
 #[no_mangle]
 pub extern "C" fn wgpu_device_poll(device_id: DeviceId, force_wait: bool) {
-    let (device_guard, mut token) = HUB.devices.read(&mut Token::root());
-    let callbacks = device_guard[device_id].maintain(force_wait, &mut token);
+    let callbacks = {
+        let (device_guard, mut token) = HUB.devices.read(&mut Token::root());
+        device_guard[device_id].maintain(force_wait, &mut token)
+    };
     Device::fire_map_callbacks(callbacks);
 }
 

wgpu-native/src/hub.rs

@@ -198,8 +198,9 @@ impl Access<TextureViewHandle> for TextureHandle {}
 impl Access<SamplerHandle> for Root {}
 impl Access<SamplerHandle> for TextureViewHandle {}
 
+#[cfg(debug_assertions)]
 thread_local! {
-    static ACTIVE_TOKEN: Cell<bool> = Cell::new(false);
+    static ACTIVE_TOKEN: Cell<u8> = Cell::new(0);
 }
 
 /// A permission token to lock resource `T` or anything after it,
@@ -209,38 +210,42 @@ thread_local! {
 /// at a time, which is enforced by `ACTIVE_TOKEN`.
 pub struct Token<'a, T: 'a> {
     level: PhantomData<&'a T>,
-    is_root: bool,
 }
 
 impl<'a, T> Token<'a, T> {
     fn new() -> Self {
+        #[cfg(debug_assertions)]
+        ACTIVE_TOKEN.with(|active| {
+            let old = active.get();
+            assert_ne!(old, 0, "Root token was dropped");
+            active.set(old + 1);
+        });
         Token {
             level: PhantomData,
-            is_root: false,
         }
     }
 }
 
 impl Token<'static, Root> {
     pub fn root() -> Self {
+        #[cfg(debug_assertions)]
         ACTIVE_TOKEN.with(|active| {
-            assert!(!active.replace(true));
+            assert_eq!(0, active.replace(1), "Root token is already active");
         });
 
         Token {
             level: PhantomData,
-            is_root: true,
         }
     }
 }
 
 impl<'a, T> Drop for Token<'a, T> {
     fn drop(&mut self) {
-        if self.is_root {
-            ACTIVE_TOKEN.with(|active| {
-                assert!(active.replace(false));
-            });
-        }
+        #[cfg(debug_assertions)]
+        ACTIVE_TOKEN.with(|active| {
+            let old = active.get();
+            active.set(old - 1);
+        });
     }
 }