Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Queue::submit should not panic on validation errors #6317

Closed
ErichDonGubler opened this issue Sep 23, 2024 · 0 comments · Fixed by #6318
Closed

Queue::submit should not panic on validation errors #6317

ErichDonGubler opened this issue Sep 23, 2024 · 0 comments · Fixed by #6318
Assignees
Labels
area: correctness We're behaving incorrectly

Comments

@ErichDonGubler
Copy link
Member

ErichDonGubler commented Sep 23, 2024

Description

When an invalid CommandBuffer is fed into Queue::submit on trunk (but, notably, not on 22.1.0), it panics on errors after they are forwarded to our fatal error machinery. This is wrong; we should be handling errors non-fatally, i.e., via the error sink set up by Device::on_uncaptured_error or the default panic handler.

Repro steps

A Cargo project with the following contents reproduces it for me:

  • Cargo.toml:

     [package]
     name = "bruh-idk"
     version = "0.1.0"
     edition = "2021"
     
     [dependencies]
     pollster = "0.3.0"
     wgpu = { git = "https://github.com/gfx-rs/wgpu", rev = "859dd8817e7484b51823d443d7cac93c6e9a7ef2" }
    
  • src/main.rs

     use pollster::FutureExt as _;
    
     fn main() {
         let instance = wgpu::Instance::new(Default::default());
     
         let adapter = instance
             .request_adapter(&Default::default())
             .block_on()
             .unwrap();
     
         let (device, queue) = adapter
             .request_device(&Default::default(), None)
             .block_on()
             .unwrap();
     
         let shader_source = "\
     @group(0) @binding(0)
     var<storage, read_write> stuff: u32;
     
     @compute @workgroup_size(1) fn main() {
         stuff = 2u;
     }
     ";
     
         let module = device.create_shader_module(wgpu::ShaderModuleDescriptor {
             label: None,
             source: wgpu::ShaderSource::Wgsl(shader_source.into()),
         });
     
         let compute_pipeline = device.create_compute_pipeline(&wgpu::ComputePipelineDescriptor {
             label: None,
             layout: None,
             module: &module,
             entry_point: None,
             compilation_options: Default::default(),
             cache: Default::default(),
         });
     
         device.push_error_scope(wgpu::ErrorFilter::Validation);
         {
             let mut command_encoder = device.create_command_encoder(&Default::default());
             {
                 let mut render_pass = command_encoder.begin_compute_pass(&Default::default());
                 render_pass.set_pipeline(&compute_pipeline);
     
                 // NOTE: We deliberately don't set a bind group here, to provoke a validation error.
     
                 render_pass.dispatch_workgroups(1, 1, 1);
             }
     
             let _idx = queue.submit([command_encoder.finish()]);
         }
     }

Expected vs observed behavior

The error scope in the device.push_error_scope from repro steps should absorb the validation errors emitted by the RenderPass::end and all subsequent operations in which the invalidation propagates, including Queue::submit. Queue::submit, however, panics instead. Observe that the above example produces the following output:

Error in Queue::submit: Validation Error

Caused by:
  Command encoder is invalid

note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Extra materials

Discovered in #5714 (comment) as a blocker for that PR.

Platform

-

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: correctness We're behaving incorrectly
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

1 participant