Skip to content

Commit a67aa54

Browse files
committed
Fix Metal wait on errored command buffers
and Add a Metal regression test covering wait_indefinitely on long-running work that can hit command-buffer error completion.
1 parent 2c81898 commit a67aa54

4 files changed

Lines changed: 105 additions & 16 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,7 @@ By @beholdnec in [#8505](https://github.com/gfx-rs/wgpu/pull/8505).
236236
#### Metal
237237

238238
- Fix crash on fence creation when running in a MacOS Seatbelt sandbox. By @wumpf in [#9415](https://github.com/gfx-rs/wgpu/pull/9415)
239+
- Fixed `Device::poll(PollType::wait_indefinitely())` hanging when a Metal command buffer exits with an error. By @39ali in [#9328](https://github.com/gfx-rs/wgpu/pull/9328).
239240
- Fixed structure field names incorrectly ignoring reserved keywords in the Metal (MSL) backend. By @39ali [#9379](https://github.com/gfx-rs/wgpu/pull/9379).
240241
- Restore the `Queue::as_raw` method, which was removed without good reason in v29. It now returns `&ProtocolObject<dyn MTLCommandQueue>`. By @andyleiserson in [#9560](https://github.com/gfx-rs/wgpu/pull/9560).
241242

tests/tests/wgpu-gpu/poll.rs

Lines changed: 94 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,20 @@
11
use std::{num::NonZeroU64, time::Duration};
22

33
use wgpu::{
4-
BindGroupDescriptor, BindGroupEntry, BindGroupLayoutDescriptor, BindGroupLayoutEntry,
4+
Backends, BindGroupDescriptor, BindGroupEntry, BindGroupLayoutDescriptor, BindGroupLayoutEntry,
55
BindingResource, BindingType, BufferBindingType, BufferDescriptor, BufferUsages, CommandBuffer,
6-
CommandEncoderDescriptor, ComputePassDescriptor, PollType, ShaderStages,
6+
CommandEncoderDescriptor, ComputePassDescriptor, ComputePipelineDescriptor,
7+
PipelineLayoutDescriptor, PollType, ShaderModuleDescriptor, ShaderSource, ShaderStages,
78
};
89

910
use wgpu_test::{
10-
gpu_test, GpuTestConfiguration, GpuTestInitializer, TestParameters, TestingContext,
11+
gpu_test, FailureCase, GpuTestConfiguration, GpuTestInitializer, TestParameters, TestingContext,
1112
};
1213

1314
pub fn all_tests(vec: &mut Vec<GpuTestInitializer>) {
1415
vec.extend([
1516
WAIT,
17+
WAIT_INDEFINITELY_LONG_RUNNING,
1618
WAIT_WITH_TIMEOUT,
1719
WAIT_WITH_TIMEOUT_MAX,
1820
DOUBLE_WAIT,
@@ -85,6 +87,95 @@ static WAIT: GpuTestConfiguration = GpuTestConfiguration::new()
8587
.unwrap();
8688
});
8789

90+
/// Regression test for <https://github.com/gfx-rs/wgpu/issues/9531>.
91+
#[gpu_test]
92+
static WAIT_INDEFINITELY_LONG_RUNNING: GpuTestConfiguration = GpuTestConfiguration::new()
93+
.parameters(
94+
TestParameters::default()
95+
.test_features_limits()
96+
.skip(FailureCase::backend(!Backends::METAL)),
97+
)
98+
.run_async(|ctx| async move {
99+
const SHADER: &str = r#"
100+
@group(0) @binding(0) var<storage, read_write> buf: array<u32>;
101+
102+
@compute @workgroup_size(64)
103+
fn main(@builtin(global_invocation_id) gid: vec3<u32>) {
104+
var x: u32 = gid.x ^ 0xDEADBEEFu;
105+
for (var i: u32 = 0u; i < 5000000u; i++) {
106+
x ^= x << 13u;
107+
x ^= x >> 17u;
108+
x ^= x << 5u;
109+
}
110+
buf[gid.x] = x;
111+
}
112+
"#;
113+
114+
const N_THREADS: u32 = 1024 * 64;
115+
116+
let module = ctx.device.create_shader_module(ShaderModuleDescriptor {
117+
label: None,
118+
source: ShaderSource::Wgsl(SHADER.into()),
119+
});
120+
let buffer = ctx.device.create_buffer(&BufferDescriptor {
121+
label: None,
122+
size: (N_THREADS as u64) * 4,
123+
usage: BufferUsages::STORAGE,
124+
mapped_at_creation: false,
125+
});
126+
let bind_group_layout = ctx
127+
.device
128+
.create_bind_group_layout(&BindGroupLayoutDescriptor {
129+
label: None,
130+
entries: &[BindGroupLayoutEntry {
131+
binding: 0,
132+
visibility: ShaderStages::COMPUTE,
133+
ty: BindingType::Buffer {
134+
ty: BufferBindingType::Storage { read_only: false },
135+
has_dynamic_offset: false,
136+
min_binding_size: None,
137+
},
138+
count: None,
139+
}],
140+
});
141+
let pipeline_layout = ctx
142+
.device
143+
.create_pipeline_layout(&PipelineLayoutDescriptor {
144+
label: None,
145+
bind_group_layouts: &[Some(&bind_group_layout)],
146+
immediate_size: 0,
147+
});
148+
let pipeline = ctx
149+
.device
150+
.create_compute_pipeline(&ComputePipelineDescriptor {
151+
label: None,
152+
layout: Some(&pipeline_layout),
153+
module: &module,
154+
entry_point: Some("main"),
155+
compilation_options: Default::default(),
156+
cache: None,
157+
});
158+
let bind_group = ctx.device.create_bind_group(&BindGroupDescriptor {
159+
label: None,
160+
layout: &bind_group_layout,
161+
entries: &[BindGroupEntry {
162+
binding: 0,
163+
resource: buffer.as_entire_binding(),
164+
}],
165+
});
166+
let mut encoder = ctx
167+
.device
168+
.create_command_encoder(&CommandEncoderDescriptor::default());
169+
{
170+
let mut cpass = encoder.begin_compute_pass(&ComputePassDescriptor::default());
171+
cpass.set_pipeline(&pipeline);
172+
cpass.set_bind_group(0, &bind_group, &[]);
173+
cpass.dispatch_workgroups(N_THREADS / 64, 1, 1);
174+
}
175+
ctx.queue.submit(Some(encoder.finish()));
176+
ctx.async_poll(PollType::wait_indefinitely()).await.unwrap();
177+
});
178+
88179
#[gpu_test]
89180
static WAIT_WITH_TIMEOUT: GpuTestConfiguration = GpuTestConfiguration::new()
90181
.parameters(TestParameters::default().enable_noop())

wgpu-hal/src/metal/device.rs

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1922,19 +1922,13 @@ impl crate::Device for super::Device {
19221922
}
19231923
}
19241924

1925-
if let Some(deadline) =
1926-
timeout.and_then(|timeout| std::time::Instant::now().checked_add(timeout))
1927-
{
1928-
while *lock < wait_value {
1929-
let result = condvar.wait_until(&mut lock, deadline);
1930-
if result.timed_out() {
1931-
return Ok(*lock >= wait_value);
1932-
}
1925+
if let Some(timeout) = timeout {
1926+
let result = condvar.wait_while_for(&mut lock, |value| *value < wait_value, timeout);
1927+
if result.timed_out() {
1928+
return Ok(*lock >= wait_value);
19331929
}
19341930
} else {
1935-
while *lock < wait_value {
1936-
condvar.wait(&mut lock);
1937-
}
1931+
condvar.wait_while(&mut lock, |value| *value < wait_value);
19381932
}
19391933

19401934
Ok(true)

wgpu-hal/src/metal/mod.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1050,8 +1050,11 @@ impl Fence {
10501050
let mut max_value = *self.sync.0.lock();
10511051
let pending_command_buffers = self.pending_command_buffers.read();
10521052
for &(value, ref cmd_buf) in pending_command_buffers.iter() {
1053-
if cmd_buf.status() == MTLCommandBufferStatus::Completed {
1054-
max_value = value;
1053+
match cmd_buf.status() {
1054+
MTLCommandBufferStatus::Completed | MTLCommandBufferStatus::Error => {
1055+
max_value = value;
1056+
}
1057+
_ => {}
10551058
}
10561059
}
10571060
max_value

0 commit comments

Comments
 (0)