Make error scopes thread local (#8585)

Author: cwfitzgerald

CHANGELOG.md

@@ -106,6 +106,16 @@ One other breaking change worth noting is that in WGSL `@builtin(view_index)` no
 
 By @SupaMaggie70Incorporated in [#8206](https://github.com/gfx-rs/wgpu/pull/8206).
 
+#### Error Scopes are now thread-local
+
+Device error scopes now operate on a per-thread basis. This allows them to be used easily within multithreaded contexts,
+without having the error scope capture errors from other threads.
+
+When the `std` feature is **not** enabled, we have no way to differentiate between threads, so error scopes return to be
+global operations.
+
+By @cwfitzgerald in [#8685](https://github.com/gfx-rs/wgpu/pull/8685)
+
 #### Log Levels
 
 We have received complaints about wgpu being way too log spammy at log levels `info`/`warn`/`error`. We have

Cargo.lock

@@ -68,6 +68,7 @@ authors = ["gfx-rs developers"]
 naga = { version = "27.0.0", path = "./naga" }
 naga-test = { path = "./naga-test" }
 wgpu = { version = "27.0.0", path = "./wgpu", default-features = false, features = [
+    "std",
     "serde",
     "wgsl",
     "vulkan",

Cargo.toml

@@ -0,0 +1,42 @@
+#![cfg(not(target_arch = "wasm32"))]
+use std::sync::{
+    atomic::{AtomicBool, Ordering},
+    Arc,
+};
+
+// Test that error scopes are thread-local: an error scope pushed on one thread
+// does not capture errors generated on another thread.
+#[test]
+fn multi_threaded_scopes() {
+    let (device, _queue) = wgpu::Device::noop(&wgpu::DeviceDescriptor::default());
+
+    let other_thread_error = Arc::new(AtomicBool::new(false));
+    let other_thread_error_clone = other_thread_error.clone();
+
+    // Start an error scope on the main thread.
+    device.push_error_scope(wgpu::ErrorFilter::Validation);
+    // Register an uncaptured error handler to catch errors from other threads.
+    device.on_uncaptured_error(Arc::new(move |_error| {
+        other_thread_error_clone.store(true, Ordering::Relaxed);
+    }));
+
+    // Do something invalid on another thread.
+    std::thread::scope(|s| {
+        s.spawn(|| {
+            let _buffer = device.create_buffer(&wgpu::BufferDescriptor {
+                label: None,
+                size: 1 << 63, // Too large!
+                usage: wgpu::BufferUsages::COPY_SRC,
+                mapped_at_creation: false,
+            });
+        });
+    });
+
+    // Pop the error scope on the main thread.
+    let error = pollster::block_on(device.pop_error_scope());
+
+    // The main thread's error scope should not have captured the other thread's error.
+    assert!(error.is_none());
+    // The other thread's error should have been reported to the uncaptured error handler.
+    assert!(other_thread_error.load(Ordering::Relaxed));
+}

tests/tests/wgpu-validation/api/error_scopes.rs

@@ -5,6 +5,7 @@ mod buffer_slice;
 mod command_buffer_actions;
 mod device;
 mod encoding;
+mod error_scopes;
 mod experimental;
 mod external_texture;
 mod instance;

tests/tests/wgpu-validation/api/mod.rs

@@ -193,6 +193,7 @@ bitflags.workspace = true
 cfg-if.workspace = true
 document-features.workspace = true
 log.workspace = true
+hashbrown.workspace = true
 parking_lot = { workspace = true, optional = true }
 profiling.workspace = true
 raw-window-handle = { workspace = true, features = ["alloc"] }

wgpu/Cargo.toml

@@ -410,12 +410,30 @@ impl Device {
         self.inner.on_uncaptured_error(handler)
     }
 
-    /// Push an error scope.
+    /// Push an error scope on this device's error scope stack.
+    /// All operations on this device, or on resources created from
+    /// this device, will have their errors captured by this scope
+    /// until a corresponding pop is made.
+    ///
+    /// Multiple error scopes may be active at one time, forming a stack.
+    /// Each error will be reported to the inner-most scope that matches
+    /// its filter.
+    ///
+    /// With the `std` feature enabled, this stack is **thread-local**.
+    /// Without, this is **global** to all threads.
     pub fn push_error_scope(&self, filter: ErrorFilter) {
         self.inner.push_error_scope(filter)
     }
 
-    /// Pop an error scope.
+    /// Pop an error scope from this device's error scope stack. Returns
+    /// a future which resolves to the error captured by this scope, if any.
+    ///
+    /// This will pop the most recently pushed error scope on this device.
+    ///
+    /// If there are no error scopes on this device, this will panic.
+    ///
+    /// With the `std` feature enabled, the error stack is **thread-local**.
+    /// Without, this is **global** to all threads.
     pub fn pop_error_scope(&self) -> impl Future<Output = Option<Error>> + WasmNotSend {
         self.inner.pop_error_scope()
     }

wgpu/src/api/device.rs

@@ -16,6 +16,7 @@ use core::{
     ptr::NonNull,
     slice,
 };
+use hashbrown::HashMap;
 
 use arrayvec::ArrayVec;
 use smallvec::SmallVec;
@@ -37,6 +38,8 @@ use crate::{
 };
 use crate::{dispatch::DispatchAdapter, util::Mutex};
 
+mod thread_id;
+
 #[derive(Clone)]
 pub struct ContextWgpuCore(Arc<wgc::global::Global>);
 
@@ -627,14 +630,14 @@ struct ErrorScope {
 }
 
 struct ErrorSinkRaw {
-    scopes: Vec<ErrorScope>,
+    scopes: HashMap<thread_id::ThreadId, Vec<ErrorScope>>,
     uncaptured_handler: Option<Arc<dyn crate::UncapturedErrorHandler>>,
 }
 
 impl ErrorSinkRaw {
     fn new() -> ErrorSinkRaw {
         ErrorSinkRaw {
-            scopes: Vec::new(),
+            scopes: HashMap::new(),
             uncaptured_handler: None,
         }
     }
@@ -656,12 +659,9 @@ impl ErrorSinkRaw {
             crate::Error::Validation { .. } => crate::ErrorFilter::Validation,
             crate::Error::Internal { .. } => crate::ErrorFilter::Internal,
         };
-        match self
-            .scopes
-            .iter_mut()
-            .rev()
-            .find(|scope| scope.filter == filter)
-        {
+        let thread_id = thread_id::ThreadId::current();
+        let scopes = self.scopes.entry(thread_id).or_default();
+        match scopes.iter_mut().rev().find(|scope| scope.filter == filter) {
             Some(scope) => {
                 if scope.error.is_none() {
                     scope.error = Some(err);
@@ -1805,15 +1805,20 @@ impl dispatch::DeviceInterface for CoreDevice {
 
     fn push_error_scope(&self, filter: crate::ErrorFilter) {
         let mut error_sink = self.error_sink.lock();
-        error_sink.scopes.push(ErrorScope {
+        let thread_id = thread_id::ThreadId::current();
+        let scopes = error_sink.scopes.entry(thread_id).or_default();
+        scopes.push(ErrorScope {
             error: None,
             filter,
         });
     }
 
     fn pop_error_scope(&self) -> Pin<Box<dyn dispatch::PopErrorScopeFuture>> {
         let mut error_sink = self.error_sink.lock();
-        let scope = error_sink.scopes.pop().unwrap();
+        let thread_id = thread_id::ThreadId::current();
+        let err = "Mismatched pop_error_scope call: no error scope for this thread. Error scopes are thread-local.";
+        let scopes = error_sink.scopes.get_mut(&thread_id).expect(err);
+        let scope = scopes.pop().expect(err);
         Box::pin(ready(scope.error))
     }
 

wgpu/src/backend/wgpu_core.rs

@@ -0,0 +1,30 @@
+//! Implementation of thread IDs for error scope tracking.
+//!
+//! Supports both std and no_std environments, though
+//! the no_std implementation is a stub that does not
+//! actually distinguish between threads.
+
+#[cfg(feature = "std")]
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
+pub struct ThreadId(std::thread::ThreadId);
+
+#[cfg(feature = "std")]
+impl ThreadId {
+    pub fn current() -> Self {
+        ThreadId(std::thread::current().id())
+    }
+}
+
+#[cfg(not(feature = "std"))]
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
+pub struct ThreadId(());
+
+#[cfg(not(feature = "std"))]
+impl ThreadId {
+    pub fn current() -> Self {
+        // A simple stub implementation for non-std environments. On
+        // no_std but multithreaded platforms, this will work, but
+        // make error scope global rather than thread-local.
+        ThreadId(())
+    }
+}