Skip to content

Commit fc02362

Browse files
committed
Convert error scopes to use thread-local guards
1 parent 75188a7 commit fc02362

File tree

15 files changed

+280
-76
lines changed

15 files changed

+280
-76
lines changed

CHANGELOG.md

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,14 @@ One other breaking change worth noting is that in WGSL `@builtin(view_index)` no
106106

107107
By @SupaMaggie70Incorporated in [#8206](https://github.com/gfx-rs/wgpu/pull/8206).
108108

109-
#### Error Scopes are now thread-local
109+
#### Error scopes now use guards and are thread local.
110+
111+
```diff
112+
- device.push_error_scope(wgpu::ErrorFilter::Validation);
113+
- device.pop_error_scope().await;
114+
+ let scope = device.push_error_scope(wgpu::ErrorFilter::Validation);
115+
+ scope.pop().await;
116+
```
110117

111118
Device error scopes now operate on a per-thread basis. This allows them to be used easily within multithreaded contexts,
112119
without having the error scope capture errors from other threads.

examples/standalone/custom_backend/src/custom.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -251,11 +251,11 @@ impl DeviceInterface for CustomDevice {
251251
unimplemented!()
252252
}
253253

254-
fn push_error_scope(&self, _filter: wgpu::ErrorFilter) {
254+
fn push_error_scope(&self, _filter: wgpu::ErrorFilter) -> u32 {
255255
unimplemented!()
256256
}
257257

258-
fn pop_error_scope(&self) -> Pin<Box<dyn wgpu::custom::PopErrorScopeFuture>> {
258+
fn pop_error_scope(&self, _index: u32) -> Pin<Box<dyn wgpu::custom::PopErrorScopeFuture>> {
259259
unimplemented!()
260260
}
261261

tests/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,4 +94,5 @@ wasm-bindgen.workspace = true
9494
web-sys = { workspace = true, features = ["CanvasRenderingContext2d", "Blob"] }
9595

9696
[lints.clippy]
97+
bool_assert_comparison = "allow"
9798
disallowed_types = "allow"

tests/src/lib.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,9 @@ pub fn fail<T>(
3939
callback: impl FnOnce() -> T,
4040
expected_msg_substring: Option<&str>,
4141
) -> T {
42-
device.push_error_scope(wgpu::ErrorFilter::Validation);
42+
let scope = device.push_error_scope(wgpu::ErrorFilter::Validation);
4343
let result = callback();
44-
let validation_error = pollster::block_on(device.pop_error_scope())
44+
let validation_error = pollster::block_on(scope.pop())
4545
.expect("expected validation error in callback, but no validation error was emitted");
4646
if let Some(expected_msg_substring) = expected_msg_substring {
4747
let lowered_expected = expected_msg_substring.to_lowercase();
@@ -63,9 +63,9 @@ pub fn fail<T>(
6363
/// Run some code in an error scope and assert that validation succeeds.
6464
#[track_caller]
6565
pub fn valid<T>(device: &wgpu::Device, callback: impl FnOnce() -> T) -> T {
66-
device.push_error_scope(wgpu::ErrorFilter::Validation);
66+
let scope = device.push_error_scope(wgpu::ErrorFilter::Validation);
6767
let result = callback();
68-
if let Some(error) = pollster::block_on(device.pop_error_scope()) {
68+
if let Some(error) = pollster::block_on(scope.pop()) {
6969
panic!(
7070
"`valid` block at {} encountered wgpu error:\n{error}",
7171
std::panic::Location::caller()
@@ -95,9 +95,9 @@ fn did_fill_error_scope<T>(
9595
callback: impl FnOnce() -> T,
9696
filter: wgpu::ErrorFilter,
9797
) -> (bool, T) {
98-
device.push_error_scope(filter);
98+
let scope = device.push_error_scope(filter);
9999
let result = callback();
100-
let validation_error = pollster::block_on(device.pop_error_scope());
100+
let validation_error = pollster::block_on(scope.pop());
101101
let failed = validation_error.is_some();
102102

103103
(failed, result)

tests/tests/wgpu-gpu/dispatch_workgroups_indirect.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ static RESET_BIND_GROUPS: GpuTestConfiguration = GpuTestConfiguration::new()
7777
}).enable_noop(),
7878
)
7979
.run_async(|ctx| async move {
80-
ctx.device.push_error_scope(wgpu::ErrorFilter::Validation);
80+
let scope = ctx.device.push_error_scope(wgpu::ErrorFilter::Validation);
8181

8282
let test_resources = TestResources::new(&ctx);
8383

@@ -98,7 +98,7 @@ static RESET_BIND_GROUPS: GpuTestConfiguration = GpuTestConfiguration::new()
9898
}
9999
ctx.queue.submit(Some(encoder.finish()));
100100

101-
let error = pollster::block_on(ctx.device.pop_error_scope());
101+
let error = pollster::block_on(scope.pop());
102102
assert!(error.is_some_and(|error| {
103103
format!("{error}").contains("The current set ComputePipeline with '' label expects a BindGroup to be set at index 0")
104104
}));
@@ -120,7 +120,7 @@ static ZERO_SIZED_BUFFER: GpuTestConfiguration = GpuTestConfiguration::new()
120120
.enable_noop(),
121121
)
122122
.run_async(|ctx| async move {
123-
ctx.device.push_error_scope(wgpu::ErrorFilter::Validation);
123+
let scope = ctx.device.push_error_scope(wgpu::ErrorFilter::Validation);
124124

125125
let test_resources = TestResources::new(&ctx);
126126

@@ -141,7 +141,7 @@ static ZERO_SIZED_BUFFER: GpuTestConfiguration = GpuTestConfiguration::new()
141141
}
142142
ctx.queue.submit(Some(encoder.finish()));
143143

144-
let error = pollster::block_on(ctx.device.pop_error_scope());
144+
let error = pollster::block_on(scope.pop());
145145
assert!(error.is_some_and(|error| {
146146
format!("{error}").contains(
147147
"Indirect buffer uses bytes 0..12 which overruns indirect buffer of size 0",

tests/tests/wgpu-gpu/oom.rs

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ static TEXTURE_OOM_TEST: GpuTestConfiguration = GpuTestConfiguration::new()
4646
.run_async(|ctx| async move {
4747
let mut textures = Vec::new();
4848
for _ in 0..LOOP_BOUND {
49-
ctx.device.push_error_scope(ErrorFilter::OutOfMemory);
49+
let scope = ctx.device.push_error_scope(ErrorFilter::OutOfMemory);
5050
let texture = ctx.device.create_texture(&TextureDescriptor {
5151
label: None,
5252
size: Extent3d {
@@ -61,7 +61,7 @@ static TEXTURE_OOM_TEST: GpuTestConfiguration = GpuTestConfiguration::new()
6161
usage: TextureUsages::RENDER_ATTACHMENT,
6262
view_formats: &[],
6363
});
64-
if let Some(err) = ctx.device.pop_error_scope().await {
64+
if let Some(err) = scope.pop().await {
6565
match err {
6666
Error::OutOfMemory { .. } => {
6767
return;
@@ -85,14 +85,14 @@ static BUFFER_OOM_TEST: GpuTestConfiguration = GpuTestConfiguration::new()
8585
.run_async(|ctx| async move {
8686
let mut buffers = Vec::new();
8787
for _ in 0..LOOP_BOUND {
88-
ctx.device.push_error_scope(ErrorFilter::OutOfMemory);
88+
let scope = ctx.device.push_error_scope(ErrorFilter::OutOfMemory);
8989
let buffer = ctx.device.create_buffer(&BufferDescriptor {
9090
label: None,
9191
size: 256 * 1024 * 1024,
9292
usage: BufferUsages::STORAGE,
9393
mapped_at_creation: false,
9494
});
95-
if let Some(err) = ctx.device.pop_error_scope().await {
95+
if let Some(err) = scope.pop().await {
9696
match err {
9797
Error::OutOfMemory { .. } => {
9898
return;
@@ -116,14 +116,14 @@ static MAPPING_BUFFER_OOM_TEST: GpuTestConfiguration = GpuTestConfiguration::new
116116
.run_async(|ctx| async move {
117117
let mut buffers = Vec::new();
118118
for _ in 0..LOOP_BOUND {
119-
ctx.device.push_error_scope(ErrorFilter::OutOfMemory);
119+
let scope = ctx.device.push_error_scope(ErrorFilter::OutOfMemory);
120120
let buffer = ctx.device.create_buffer(&BufferDescriptor {
121121
label: None,
122122
size: 256 * 1024 * 1024,
123123
usage: BufferUsages::COPY_SRC | BufferUsages::MAP_WRITE,
124124
mapped_at_creation: false,
125125
});
126-
if let Some(err) = ctx.device.pop_error_scope().await {
126+
if let Some(err) = scope.pop().await {
127127
match err {
128128
Error::OutOfMemory { .. } => {
129129
return;
@@ -148,13 +148,13 @@ static QUERY_SET_OOM_TEST: GpuTestConfiguration = GpuTestConfiguration::new()
148148
.run_async(|ctx| async move {
149149
let mut query_sets = Vec::new();
150150
for _ in 0..LOOP_BOUND {
151-
ctx.device.push_error_scope(ErrorFilter::OutOfMemory);
151+
let scope = ctx.device.push_error_scope(ErrorFilter::OutOfMemory);
152152
let query_set = ctx.device.create_query_set(&QuerySetDescriptor {
153153
label: None,
154154
ty: QueryType::Occlusion,
155155
count: 4096,
156156
});
157-
if let Some(err) = ctx.device.pop_error_scope().await {
157+
if let Some(err) = scope.pop().await {
158158
match err {
159159
Error::OutOfMemory { .. } => {
160160
return;
@@ -179,7 +179,7 @@ static BLAS_OOM_TEST: GpuTestConfiguration = GpuTestConfiguration::new()
179179
.run_async(|ctx| async move {
180180
let mut blases = Vec::new();
181181
for _ in 0..LOOP_BOUND {
182-
ctx.device.push_error_scope(ErrorFilter::OutOfMemory);
182+
let scope = ctx.device.push_error_scope(ErrorFilter::OutOfMemory);
183183
let blas = ctx.device.create_blas(
184184
&CreateBlasDescriptor {
185185
label: None,
@@ -196,7 +196,7 @@ static BLAS_OOM_TEST: GpuTestConfiguration = GpuTestConfiguration::new()
196196
}],
197197
},
198198
);
199-
if let Some(err) = ctx.device.pop_error_scope().await {
199+
if let Some(err) = scope.pop().await {
200200
match err {
201201
Error::OutOfMemory { .. } => {
202202
return;
@@ -221,14 +221,14 @@ static TLAS_OOM_TEST: GpuTestConfiguration = GpuTestConfiguration::new()
221221
.run_async(|ctx| async move {
222222
let mut tlases = Vec::new();
223223
for _ in 0..LOOP_BOUND {
224-
ctx.device.push_error_scope(ErrorFilter::OutOfMemory);
224+
let scope = ctx.device.push_error_scope(ErrorFilter::OutOfMemory);
225225
let tlas = ctx.device.create_tlas(&CreateTlasDescriptor {
226226
label: None,
227227
max_instances: 1024 * 1024,
228228
flags: AccelerationStructureFlags::PREFER_FAST_TRACE,
229229
update_mode: AccelerationStructureUpdateMode::Build,
230230
});
231-
if let Some(err) = ctx.device.pop_error_scope().await {
231+
if let Some(err) = scope.pop().await {
232232
match err {
233233
Error::OutOfMemory { .. } => {
234234
return;

tests/tests/wgpu-gpu/query_set.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ static DROP_FAILED_TIMESTAMP_QUERY_SET: GpuTestConfiguration = GpuTestConfigurat
1010
.run_sync(|ctx| {
1111
// Enter an error scope, so the validation catch-all doesn't
1212
// report the error too early.
13-
ctx.device.push_error_scope(wgpu::ErrorFilter::Validation);
13+
let scope = ctx.device.push_error_scope(wgpu::ErrorFilter::Validation);
1414

1515
// Creating this query set should fail, since we didn't include
1616
// TIMESTAMP_QUERY in our required features.
@@ -23,5 +23,5 @@ static DROP_FAILED_TIMESTAMP_QUERY_SET: GpuTestConfiguration = GpuTestConfigurat
2323
// Dropping this should not panic.
2424
drop(bad_query_set);
2525

26-
assert!(pollster::block_on(ctx.device.pop_error_scope()).is_some());
26+
assert!(pollster::block_on(scope.pop()).is_some());
2727
});

tests/tests/wgpu-gpu/shader/compilation_messages/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,11 @@ static SHADER_COMPILE_SUCCESS: GpuTestConfiguration = GpuTestConfiguration::new(
2424
static SHADER_COMPILE_ERROR: GpuTestConfiguration = GpuTestConfiguration::new()
2525
.parameters(TestParameters::default().enable_noop())
2626
.run_async(|ctx| async move {
27-
ctx.device.push_error_scope(wgpu::ErrorFilter::Validation);
27+
let scope = ctx.device.push_error_scope(wgpu::ErrorFilter::Validation);
2828
let sm = ctx
2929
.device
3030
.create_shader_module(include_wgsl!("error_shader.wgsl"));
31-
assert!(pollster::block_on(ctx.device.pop_error_scope()).is_some());
31+
assert!(pollster::block_on(scope.pop()).is_some());
3232

3333
let compilation_info = sm.get_compilation_info().await;
3434
let error_message = compilation_info
Lines changed: 91 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,42 +1,116 @@
11
#![cfg(not(target_arch = "wasm32"))]
2-
use std::sync::{
3-
atomic::{AtomicBool, Ordering},
4-
Arc,
2+
use std::{
3+
panic::AssertUnwindSafe,
4+
sync::{
5+
atomic::{AtomicBool, Ordering},
6+
Arc,
7+
},
58
};
69

10+
fn raise_validation_error(device: &wgpu::Device) {
11+
let _buffer = device.create_buffer(&wgpu::BufferDescriptor {
12+
label: None,
13+
size: 1 << 63, // Too large!
14+
usage: wgpu::BufferUsages::COPY_SRC,
15+
mapped_at_creation: false,
16+
});
17+
}
18+
19+
fn register_uncaptured_error_handler(device: &wgpu::Device) -> Arc<AtomicBool> {
20+
let error_occurred = Arc::new(AtomicBool::new(false));
21+
let error_occurred_clone = error_occurred.clone();
22+
23+
device.on_uncaptured_error(Arc::new(move |_error| {
24+
error_occurred_clone.store(true, Ordering::Relaxed);
25+
}));
26+
27+
error_occurred
28+
}
29+
30+
// Test that error scopes work correctly in the basic case.
31+
#[test]
32+
fn basic() {
33+
let (device, _queue) = wgpu::Device::noop(&wgpu::DeviceDescriptor::default());
34+
35+
let scope = device.push_error_scope(wgpu::ErrorFilter::Validation);
36+
raise_validation_error(&device);
37+
let error = pollster::block_on(scope.pop());
38+
39+
assert!(error.is_some());
40+
}
41+
742
// Test that error scopes are thread-local: an error scope pushed on one thread
843
// does not capture errors generated on another thread.
944
#[test]
1045
fn multi_threaded_scopes() {
1146
let (device, _queue) = wgpu::Device::noop(&wgpu::DeviceDescriptor::default());
1247

13-
let other_thread_error = Arc::new(AtomicBool::new(false));
14-
let other_thread_error_clone = other_thread_error.clone();
15-
1648
// Start an error scope on the main thread.
17-
device.push_error_scope(wgpu::ErrorFilter::Validation);
49+
let scope = device.push_error_scope(wgpu::ErrorFilter::Validation);
1850
// Register an uncaptured error handler to catch errors from other threads.
19-
device.on_uncaptured_error(Arc::new(move |_error| {
20-
other_thread_error_clone.store(true, Ordering::Relaxed);
21-
}));
51+
let other_thread_error = register_uncaptured_error_handler(&device);
2252

2353
// Do something invalid on another thread.
2454
std::thread::scope(|s| {
2555
s.spawn(|| {
26-
let _buffer = device.create_buffer(&wgpu::BufferDescriptor {
27-
label: None,
28-
size: 1 << 63, // Too large!
29-
usage: wgpu::BufferUsages::COPY_SRC,
30-
mapped_at_creation: false,
31-
});
56+
raise_validation_error(&device);
3257
});
3358
});
3459

3560
// Pop the error scope on the main thread.
36-
let error = pollster::block_on(device.pop_error_scope());
61+
let error = pollster::block_on(scope.pop());
3762

3863
// The main thread's error scope should not have captured the other thread's error.
3964
assert!(error.is_none());
4065
// The other thread's error should have been reported to the uncaptured error handler.
4166
assert!(other_thread_error.load(Ordering::Relaxed));
4267
}
68+
69+
// Test that error scopes error when popped in the wrong order.
70+
#[test]
71+
#[should_panic(expected = "error scopes must be popped in reverse order")]
72+
fn pop_out_of_order() {
73+
let (device, _queue) = wgpu::Device::noop(&wgpu::DeviceDescriptor::default());
74+
75+
let scope1 = device.push_error_scope(wgpu::ErrorFilter::Validation);
76+
let _scope2 = device.push_error_scope(wgpu::ErrorFilter::Validation);
77+
78+
let _ = pollster::block_on(scope1.pop());
79+
}
80+
81+
// Test that error scopes are automatically popped when dropped.
82+
#[test]
83+
fn drop_automatically_pops() {
84+
let (device, _queue) = wgpu::Device::noop(&wgpu::DeviceDescriptor::default());
85+
86+
let uncaptured_error = register_uncaptured_error_handler(&device);
87+
88+
let scope = device.push_error_scope(wgpu::ErrorFilter::Validation);
89+
raise_validation_error(&device);
90+
drop(scope); // Automatically pops the error scope.
91+
92+
assert_eq!(uncaptured_error.load(Ordering::Relaxed), false);
93+
94+
// Raising another error will go to the uncaptured error handler, not the dropped scope.
95+
raise_validation_error(&device);
96+
97+
assert_eq!(uncaptured_error.load(Ordering::Relaxed), true);
98+
}
99+
100+
// Test that error scopes are automatically popped when dropped during unwinding,
101+
// even when they are dropped out of order.
102+
#[test]
103+
fn drop_during_unwind() {
104+
let (device, _queue) = wgpu::Device::noop(&wgpu::DeviceDescriptor::default());
105+
106+
let scope1 = device.push_error_scope(wgpu::ErrorFilter::Validation);
107+
let scope2 = device.push_error_scope(wgpu::ErrorFilter::Validation);
108+
109+
let _ = std::panic::catch_unwind(AssertUnwindSafe(|| {
110+
raise_validation_error(&device);
111+
// Move scope1 so that it is dropped before scope2.
112+
let _scope2 = scope2;
113+
let _scope1 = scope1;
114+
panic!("trigger unwind");
115+
}));
116+
}

0 commit comments

Comments
 (0)