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

Always free staging buffers #2961

Merged
merged 4 commits into from
Aug 14, 2022

Conversation

jimblandy
Copy link
Member

@jimblandy jimblandy commented Aug 13, 2022

Checklist

  • Run cargo clippy.
  • Run RUSTFLAGS=--cfg=web_sys_unstable_apis cargo clippy --target wasm32-unknown-unknown if applicable.
  • Add change to CHANGELOG.md. See simple instructions inside file.

Connections
Fixes #2959.

Description
Some platform validation requires that all resources created using a given Device must be freed before the Device itself. Some operations that use StagingBuffers drop them on the ground if an error occurs.

This patch ensures that the staging buffer is not leaked when an error occurs, by allocating it as late as possible, and freeing it explicitly when those fallible operations we can't move its allocation past go awry.

Testing
Tests included in wgpu/tests.

@jimblandy jimblandy force-pushed the always-free-staging-buffers branch from 8a830d5 to d6b544f Compare August 13, 2022 00:42
This helps the borrow checker understand that we can borrow
`pending_writes`'s encoder and the raw Device at the same time.
@jimblandy jimblandy force-pushed the always-free-staging-buffers branch from d6b544f to 0d8ce02 Compare August 14, 2022 00:12
Copy link
Member

@cwfitzgerald cwfitzgerald left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love the tests, code looks good

CHANGELOG.md Outdated Show resolved Hide resolved
@jimblandy jimblandy force-pushed the always-free-staging-buffers branch 2 times, most recently from 91dcf10 to 41f287c Compare August 14, 2022 00:51
We must ensure that the staging buffer is not leaked when an error
occurs, so allocate it as late as possible, and free it explicitly when
those fallible operations we can't move it past go awry.

Fixes gfx-rs#2959.
@jimblandy jimblandy force-pushed the always-free-staging-buffers branch from 41f287c to 9022850 Compare August 14, 2022 00:58
@jimblandy jimblandy merged commit c6ca2a7 into gfx-rs:master Aug 14, 2022
@jimblandy jimblandy deleted the always-free-staging-buffers branch August 14, 2022 01:25
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.

Test case provokes Vulkan validation error
2 participants