Device::drop doesn't actually free the device when using backend::direct::Context
#2563
Labels
area: api
Issues related to API surface
Comments
jimblandy
added a commit
to jimblandy/wgpu
that referenced
this issue
Mar 31, 2022
Go ahead and call `global.device_drop` from `direct::Context::device_drop`. Let `Global::device_drop` simply drop the life guard's `RefCount`, and put off everything else entailed in freeing a device until `Device::maintain` says its queue is empty and there are no more references to it. (The user can reach that function, even after dropping their `Device`, by calling `wgpu::Instance::poll_all`.) Fixes gfx-rs#2563.
jimblandy
added a commit
to jimblandy/wgpu
that referenced
this issue
Mar 31, 2022
Go ahead and call `global.device_drop` from `direct::Context::device_drop`. Let `Global::device_drop` simply drop the life guard's `RefCount`, and put off everything else entailed in freeing a device until `Device::maintain` says its queue is empty and there are no more references to it. (The user can reach that function, even after dropping their `Device`, by calling `wgpu::Instance::poll_all`.) Fixes gfx-rs#2563.
jimblandy
added a commit
to jimblandy/wgpu
that referenced
this issue
Mar 31, 2022
Go ahead and call `global.device_drop` from `direct::Context::device_drop`. Let `Global::device_drop` simply drop the life guard's `RefCount`, and put off everything else entailed in freeing a device until `Device::maintain` says its queue is empty and there are no more references to it. (The user can reach that function, even after dropping their `Device`, by calling `wgpu::Instance::poll_all`.) Fixes gfx-rs#2563.
kvark
pushed a commit
that referenced
this issue
Apr 1, 2022
Go ahead and call `global.device_drop` from `direct::Context::device_drop`. Let `Global::device_drop` simply drop the life guard's `RefCount`, and put off everything else entailed in freeing a device until `Device::maintain` says its queue is empty and there are no more references to it. (The user can reach that function, even after dropping their `Device`, by calling `wgpu::Instance::poll_all`.) Fixes #2563.
moz-v2v-gh
pushed a commit
to mozilla/gecko-dev
that referenced
this issue
Apr 6, 2022
…r=jgilbert New versions of several crates are introduced to third_party/rust, by changing the versions requested in `gfx/wgpu_bindings/Cargo.toml` and running `mach vendor rust`: - `wgpu-core`, `wgpu-hal`, and `wgpu-types`, as used by `wgpu_bindings` - `naga`, `ash`, and `metal`, as used by the above These are all exact copies of the upstream sources, at the git revisions listed in `.cargo/config.in`. This brings in fixes for some upstream `wgpu` bugs that were fuzzblockers: - Compute pipelines never freed at runtime, leaking memory #2564 gfx-rs/wgpu#2564 - Device::drop doesn't actually free the device when using backend::direct::Context #2563 gfx-rs/wgpu#2563 The Firefox sources also needed some adjustments to catch up with upstream changes: - The C type `mozilla::webgpu::ffi::WGPUTextureFormat` is now a struct containing a tag enum and a union, not just an enum. This is needed for [gfx-rs/wgpu#2477](gfx-rs/wgpu#2477). (Note that Firefox's `WebGPU.webidl` is behind the current spec, so even though the newest ASTC texture formats are supported in `wgpu`, they're not available in Firefox yet.) - `wgpu` got a new feature, `id32`, which cbindgen needed to be told about so that it would generate preprocessor-protected code like this: #if defined(WGPU_FEATURE_ID32) typedef uint32_t WGPUNonZeroId; #endif #if !defined(WGPU_FEATURE_ID32) typedef uint64_t WGPUNonZeroId; #endif instead of just spitting out two conflicting definitions of `WGPUNonZeroId`. - The `wgpu_core::hub::IdentityHandlerFactory` trait's `spawn` method no longer takes a `min_index` argument. (Our implementations of that trait never used that argument anyway, so this was easy to accommodate.) Differential Revision: https://phabricator.services.mozilla.com/D142779
jamienicol
pushed a commit
to jamienicol/gecko
that referenced
this issue
Apr 6, 2022
…r=jgilbert New versions of several crates are introduced to third_party/rust, by changing the versions requested in `gfx/wgpu_bindings/Cargo.toml` and running `mach vendor rust`: - `wgpu-core`, `wgpu-hal`, and `wgpu-types`, as used by `wgpu_bindings` - `naga`, `ash`, and `metal`, as used by the above These are all exact copies of the upstream sources, at the git revisions listed in `.cargo/config.in`. This brings in fixes for some upstream `wgpu` bugs that were fuzzblockers: - Compute pipelines never freed at runtime, leaking memory #2564 gfx-rs/wgpu#2564 - Device::drop doesn't actually free the device when using backend::direct::Context #2563 gfx-rs/wgpu#2563 The Firefox sources also needed some adjustments to catch up with upstream changes: - The C type `mozilla::webgpu::ffi::WGPUTextureFormat` is now a struct containing a tag enum and a union, not just an enum. This is needed for [gfx-rs/wgpu#2477](gfx-rs/wgpu#2477). (Note that Firefox's `WebGPU.webidl` is behind the current spec, so even though the newest ASTC texture formats are supported in `wgpu`, they're not available in Firefox yet.) - `wgpu` got a new feature, `id32`, which cbindgen needed to be told about so that it would generate preprocessor-protected code like this: #if defined(WGPU_FEATURE_ID32) typedef uint32_t WGPUNonZeroId; #endif #if !defined(WGPU_FEATURE_ID32) typedef uint64_t WGPUNonZeroId; #endif instead of just spitting out two conflicting definitions of `WGPUNonZeroId`. - The `wgpu_core::hub::IdentityHandlerFactory` trait's `spawn` method no longer takes a `min_index` argument. (Our implementations of that trait never used that argument anyway, so this was easy to accommodate.) Differential Revision: https://phabricator.services.mozilla.com/D142779
moz-v2v-gh
pushed a commit
to mozilla/gecko-dev
that referenced
this issue
Apr 7, 2022
…r=jgilbert New versions of several crates are introduced to third_party/rust, by changing the versions requested in `gfx/wgpu_bindings/Cargo.toml` and running `mach vendor rust`: - `wgpu-core`, `wgpu-hal`, and `wgpu-types`, as used by `wgpu_bindings` - `naga`, `ash`, and `metal`, as used by the above These are all exact copies of the upstream sources, at the git revisions listed in `.cargo/config.in`. This brings in fixes for some upstream `wgpu` bugs that were fuzzblockers: - Compute pipelines never freed at runtime, leaking memory #2564 gfx-rs/wgpu#2564 - Device::drop doesn't actually free the device when using backend::direct::Context #2563 gfx-rs/wgpu#2563 The Firefox sources also needed some adjustments to catch up with upstream changes: - The C type `mozilla::webgpu::ffi::WGPUTextureFormat` is now a struct containing a tag enum and a union, not just an enum. This is needed for [gfx-rs/wgpu#2477](gfx-rs/wgpu#2477). (Note that Firefox's `WebGPU.webidl` is behind the current spec, so even though the newest ASTC texture formats are supported in `wgpu`, they're not available in Firefox yet.) - `wgpu` got a new feature, `id32`, which cbindgen needed to be told about so that it would generate preprocessor-protected code like this: #if defined(WGPU_FEATURE_ID32) typedef uint32_t WGPUNonZeroId; #endif #if !defined(WGPU_FEATURE_ID32) typedef uint64_t WGPUNonZeroId; #endif instead of just spitting out two conflicting definitions of `WGPUNonZeroId`. - The `wgpu_core::hub::IdentityHandlerFactory` trait's `spawn` method no longer takes a `min_index` argument. (Our implementations of that trait never used that argument anyway, so this was easy to accommodate.) Differential Revision: https://phabricator.services.mozilla.com/D142779
jamienicol
pushed a commit
to jamienicol/gecko
that referenced
this issue
Apr 7, 2022
…r=jgilbert New versions of several crates are introduced to third_party/rust, by changing the versions requested in `gfx/wgpu_bindings/Cargo.toml` and running `mach vendor rust`: - `wgpu-core`, `wgpu-hal`, and `wgpu-types`, as used by `wgpu_bindings` - `naga`, `ash`, and `metal`, as used by the above These are all exact copies of the upstream sources, at the git revisions listed in `.cargo/config.in`. This brings in fixes for some upstream `wgpu` bugs that were fuzzblockers: - Compute pipelines never freed at runtime, leaking memory #2564 gfx-rs/wgpu#2564 - Device::drop doesn't actually free the device when using backend::direct::Context #2563 gfx-rs/wgpu#2563 The Firefox sources also needed some adjustments to catch up with upstream changes: - The C type `mozilla::webgpu::ffi::WGPUTextureFormat` is now a struct containing a tag enum and a union, not just an enum. This is needed for [gfx-rs/wgpu#2477](gfx-rs/wgpu#2477). (Note that Firefox's `WebGPU.webidl` is behind the current spec, so even though the newest ASTC texture formats are supported in `wgpu`, they're not available in Firefox yet.) - `wgpu` got a new feature, `id32`, which cbindgen needed to be told about so that it would generate preprocessor-protected code like this: #if defined(WGPU_FEATURE_ID32) typedef uint32_t WGPUNonZeroId; #endif #if !defined(WGPU_FEATURE_ID32) typedef uint64_t WGPUNonZeroId; #endif instead of just spitting out two conflicting definitions of `WGPUNonZeroId`. - The `wgpu_core::hub::IdentityHandlerFactory` trait's `spawn` method no longer takes a `min_index` argument. (Our implementations of that trait never used that argument anyway, so this was easy to accommodate.) Differential Revision: https://phabricator.services.mozilla.com/D142779
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
wgpu::Device::dropcallsContext::device_drop. Onbackend::direct::Context, the implementation doesn't actually callwgpu_core::hub::Global::device_drop, so the backend device resources are never freed.The body of
backend::direct::Context::device_drophas aTODOfor this:So I guess I'm filing this issue to promote that TODO to an actual bug.
The text was updated successfully, but these errors were encountered: