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

[core] Ensure all lock guards are properly nested. #5646

Merged
merged 4 commits into from
May 3, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 0 additions & 2 deletions wgpu-core/src/device/global.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2033,7 +2033,6 @@ impl Global {
// Wait for all work to finish before configuring the surface.
let snatch_guard = device.snatchable_lock.read();
let fence = device.fence.read();
let fence = fence.as_ref().unwrap();
match device.maintain(fence, wgt::Maintain::Wait, snatch_guard) {
Ok((closures, _)) => {
user_callbacks = closures;
Expand Down Expand Up @@ -2146,7 +2145,6 @@ impl Global {
) -> Result<DevicePoll, WaitIdleError> {
let snatch_guard = device.snatchable_lock.read();
let fence = device.fence.read();
let fence = fence.as_ref().unwrap();
let (closures, queue_empty) = device.maintain(fence, maintain, snatch_guard)?;

// Some deferred destroys are scheduled in maintain so run this right after
Expand Down
32 changes: 18 additions & 14 deletions wgpu-core/src/device/queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use crate::{
hal_label,
id::{self, DeviceId, QueueId},
init_tracker::{has_copy_partial_init_tracker_coverage, TextureInitRange},
lock::{rank, Mutex},
lock::{rank, Mutex, RwLockWriteGuard},
resource::{
Buffer, BufferAccessError, BufferMapState, DestroyedBuffer, DestroyedTexture, Resource,
ResourceInfo, ResourceType, StagingBuffer, Texture, TextureInner,
Expand Down Expand Up @@ -1162,8 +1162,8 @@ impl Global {
let snatch_guard = device.snatchable_lock.read();

// Fence lock must be acquired after the snatch lock everywhere to avoid deadlocks.
let mut fence = device.fence.write();
let fence = fence.as_mut().unwrap();
let mut fence_guard = device.fence.write();
let fence = fence_guard.as_mut().unwrap();
ErichDonGubler marked this conversation as resolved.
Show resolved Hide resolved
let submit_index = device
.active_submission_index
.fetch_add(1, Ordering::Relaxed)
Expand Down Expand Up @@ -1459,8 +1459,8 @@ impl Global {
}
}

let mut pending_writes = device.pending_writes.lock();
let pending_writes = pending_writes.as_mut().unwrap();
let mut pending_writes_guard = device.pending_writes.lock();
let pending_writes = pending_writes_guard.as_mut().unwrap();

{
used_surface_textures.set_size(hub.textures.read().len());
Expand Down Expand Up @@ -1550,18 +1550,22 @@ impl Global {
active_executions,
);

// This will schedule destruction of all resources that are no longer needed
// by the user but used in the command stream, among other things.
let (closures, _) = match device.maintain(fence, wgt::Maintain::Poll, snatch_guard) {
Ok(closures) => closures,
Err(WaitIdleError::Device(err)) => return Err(QueueSubmitError::Queue(err)),
Err(WaitIdleError::StuckGpu) => return Err(QueueSubmitError::StuckGpu),
Err(WaitIdleError::WrongSubmissionIndex(..)) => unreachable!(),
};

// pending_write_resources has been drained, so it's empty, but we
// want to retain its heap allocation.
pending_writes.temp_resources = pending_write_resources;
drop(pending_writes_guard);

// This will schedule destruction of all resources that are no longer needed
// by the user but used in the command stream, among other things.
let fence_guard = RwLockWriteGuard::downgrade(fence_guard);
let (closures, _) =
match device.maintain(fence_guard, wgt::Maintain::Poll, snatch_guard) {
Ok(closures) => closures,
Err(WaitIdleError::Device(err)) => return Err(QueueSubmitError::Queue(err)),
Err(WaitIdleError::StuckGpu) => return Err(QueueSubmitError::StuckGpu),
Err(WaitIdleError::WrongSubmissionIndex(..)) => unreachable!(),
};

device.lock_life().post_submit();

(submit_index, closures)
Expand Down
4 changes: 3 additions & 1 deletion wgpu-core/src/device/resource.rs
Original file line number Diff line number Diff line change
Expand Up @@ -397,11 +397,12 @@ impl<A: HalApi> Device<A> {
/// return it to our callers.)
pub(crate) fn maintain<'this>(
&'this self,
fence: &A::Fence,
fence_guard: crate::lock::RwLockReadGuard<Option<A::Fence>>,
maintain: wgt::Maintain<queue::WrappedSubmissionIndex>,
snatch_guard: SnatchGuard,
) -> Result<(UserClosures, bool), WaitIdleError> {
profiling::scope!("Device::maintain");
let fence = fence_guard.as_ref().unwrap();
let last_done_index = if maintain.is_wait() {
let index_to_wait_for = match maintain {
wgt::Maintain::WaitForSubmissionIndex(submission_index) => {
Expand Down Expand Up @@ -481,6 +482,7 @@ impl<A: HalApi> Device<A> {

// Don't hold the locks while calling release_gpu_resources.
drop(life_tracker);
drop(fence_guard);
drop(snatch_guard);

if should_release_gpu_resource {
Expand Down