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

Make buffer_map and buffer_unmap check for device validity, add tests. #4212

Merged
merged 4 commits into from
Oct 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ By @teoxoy in [#4185](https://github.com/gfx-rs/wgpu/pull/4185)
- `wgpu::CreateSurfaceError` and `wgpu::RequestDeviceError` now give details of the failure, but no longer implement `PartialEq` and cannot be constructed. By @kpreid in [#4066](https://github.com/gfx-rs/wgpu/pull/4066) and [#4145](https://github.com/gfx-rs/wgpu/pull/4145)
- Make `WGPU_POWER_PREF=none` a valid value. By @fornwall in [4076](https://github.com/gfx-rs/wgpu/pull/4076)
- Support dual source blending in OpenGL ES, Metal, Vulkan & DX12. By @freqmod in [4022](https://github.com/gfx-rs/wgpu/pull/4022)
- Add stub support for device destroy and device validity. By @bradwerth in [4163](https://github.com/gfx-rs/wgpu/pull/4163)
- Add stub support for device destroy and device validity. By @bradwerth in [4163](https://github.com/gfx-rs/wgpu/pull/4163) and in [4212](https://github.com/gfx-rs/wgpu/pull/4212)
- Add trace-level logging for most entry points in wgpu-core By @nical in [4183](https://github.com/gfx-rs/wgpu/pull/4183)
- Add `Rgb10a2Uint` format. By @teoxoy in [4199](https://github.com/gfx-rs/wgpu/pull/4199)
- Validate that resources are used on the right device. By @nical in [4207](https://github.com/gfx-rs/wgpu/pull/4207)
Expand Down
32 changes: 27 additions & 5 deletions tests/tests/device.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,18 @@ fn device_destroy_then_more() {
usage: wgpu::BufferUsages::MAP_READ | wgpu::BufferUsages::COPY_DST,
mapped_at_creation: false,
});
let buffer_for_map = ctx.device.create_buffer(&wgpu::BufferDescriptor {
label: None,
size: 256,
usage: wgpu::BufferUsages::MAP_WRITE | wgpu::BufferUsages::COPY_SRC,
mapped_at_creation: false,
});
let buffer_for_unmap = ctx.device.create_buffer(&wgpu::BufferDescriptor {
label: None,
size: 256,
usage: wgpu::BufferUsages::MAP_WRITE | wgpu::BufferUsages::COPY_SRC,
mapped_at_creation: true,
});

// Create a bind group layout.
let bind_group_layout =
Expand Down Expand Up @@ -215,16 +227,14 @@ fn device_destroy_then_more() {
ctx.device.destroy();

// TODO: verify the following operations will return an invalid device error:
// * Run a compute pass
// * Run a render pass
// * Run a compute or render pass
// * Finish a render bundle encoder
// * Create a texture from HAL
// * Create a buffer from HAL
// * Create a sampler
// * Validate a surface configuration
// * Start capture
// * Stop capture
// * Buffer map
// * Start or stop capture
// * Get or set buffer sub data

// TODO: figure out how to structure a test around these operations which panic when
// the device is invalid:
Expand Down Expand Up @@ -439,6 +449,18 @@ fn device_destroy_then_more() {
entry_point: "",
});
});

// Buffer map should fail.
fail(&ctx.device, || {
buffer_for_map
.slice(..)
.map_async(wgpu::MapMode::Write, |_| ());
});

// Buffer unmap should fail.
fail(&ctx.device, || {
buffer_for_unmap.unmap();
});
},
)
}
15 changes: 14 additions & 1 deletion wgpu-core/src/device/global.rs
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,9 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
let device = device_guard
.get(device_id)
.map_err(|_| DeviceError::Invalid)?;
if !device.valid {
return Err(DeviceError::Invalid.into());
}
let buffer = buffer_guard
.get_mut(buffer_id)
.map_err(|_| BufferAccessError::Invalid)?;
Expand Down Expand Up @@ -436,6 +439,9 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
let device = device_guard
.get(device_id)
.map_err(|_| DeviceError::Invalid)?;
if !device.valid {
return Err(DeviceError::Invalid.into());
}
let buffer = buffer_guard
.get_mut(buffer_id)
.map_err(|_| BufferAccessError::Invalid)?;
Expand Down Expand Up @@ -2400,6 +2406,9 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
let mut token = Token::root();
let (device_guard, mut token) = hub.devices.read(&mut token);
let device = device_guard.get(device_id).map_err(|_| InvalidDevice)?;
if !device.valid {
return Err(InvalidDevice);
}
device.lock_life(&mut token).triage_suspected(
hub,
&device.trackers,
Expand Down Expand Up @@ -2711,7 +2720,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {

let device = &device_guard[buffer.device_id.value];
if !device.valid {
return Err((op, BufferAccessError::Invalid));
return Err((op, DeviceError::Invalid.into()));
}

if let Err(e) = check_buffer_usage(buffer.usage, pub_usage) {
Expand Down Expand Up @@ -2766,6 +2775,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
};

let device = &device_guard[device_id];
// Validity of device was confirmed in the code block that set device_id.
device
.lock_life(&mut token)
.map(id::Valid(buffer_id), ref_count);
Expand Down Expand Up @@ -2955,6 +2965,9 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
.get_mut(buffer_id)
.map_err(|_| BufferAccessError::Invalid)?;
let device = &mut device_guard[buffer.device_id.value];
if !device.valid {
return Err(DeviceError::Invalid.into());
}

closure = self.buffer_unmap_inner(buffer_id, buffer, device)
}
Expand Down