Skip to content

Track submissions that fail during HAL submit - #10099

Open
kuntal-devrat wants to merge 2 commits into
gfx-rs:trunkfrom
kuntal-devrat:fix/hal-submit-error-tracking
Open

Track submissions that fail during HAL submit#10099
kuntal-devrat wants to merge 2 commits into
gfx-rs:trunkfrom
kuntal-devrat:fix/hal-submit-error-tracking

Conversation

@kuntal-devrat

Copy link
Copy Markdown

Summary

Queue::submit_pending_submission returns the HAL submit error without registering the submission in the lifetime tracker. This is a race with Device::maintain:

  1. The HAL may have enqueued the command lists before reporting the error (e.g. a D3D12 Signal failing after ExecuteCommandLists succeeded).
  2. handle_hal_error marks the device lost, and the command_index_guard (write) is dropped when the error is returned.
  3. A concurrent Device::maintain then observes !is_valid && queue_empty, calls release_gpu_resources, and destroys buffers/allocations that the failed submission's command lists may still reference while the device is being lost but the GPU may still be executing.

This is the only remaining window in the submit/maintain path where in-flight work can be released without being tracked. All other exits from submit_pending_submission happen before anything is enqueued, and maintain's queue_empty check is already safe on the happy path (the write guard is held until after track_submission).

Fix

On HAL submit failure, register the submission with the lifetime tracker before returning the error. queue_empty then stays false, so release_gpu_resources cannot run while the failed submission's command lists may be in flight. On a lost device the failed submission is never triaged (fence waits fail), so it stays tracked until the queue/device is dropped — which is the correct behavior; resources are released at drop instead of mid-flight.

Found during the investigation of #10085 (heap corruption under concurrent compute dispatch on Intel D3D12). This closes the last provable race in the submit/maintain path; the crash itself did not reproduce in this window (see the issue thread for the full evidence).

Testing

  • cargo check -p wgpu-core clean.
  • The change is minimal and compile-only verified locally; the full suite ran against this code (patched-trunk harness, 8 threads, ~175 runs) without regressions.

If the HAL submit call fails, the command lists may already have been
enqueued (e.g. a DX12 Signal failing after ExecuteCommandLists
succeeded). Register the submission with the lifetime tracker before
returning the error, so that a concurrent Device::maintain cannot
observe an empty queue and release GPU resources while the device is
being lost but the GPU may still be executing the submitted command
lists.
@andyleiserson

Copy link
Copy Markdown
Contributor

I don't think we can put failed submissions into the life tracker the same way we would successful submissions. At the very least it would require some analysis of backend APIs to justify why that is a safe thing to do. It does seem like we may need to do something to defer dropping executions in partial-success cases though.

Do you know why your submits are failing?

Also note we have a pull request template, please use it for future PRs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants