diff --git a/CHANGELOG.md b/CHANGELOG.md index 593615066be..54c858ac43f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -221,6 +221,7 @@ By @beholdnec in [#8505](https://github.com/gfx-rs/wgpu/pull/8505). - Increase recursion limits to please `-Znext-solver`. By @nazar-pc in [#9609](https://github.com/gfx-rs/wgpu/pull/9609) - Stencil clear and reference values are now truncated to 8 bits. By @beicause in [#9607](https://github.com/gfx-rs/wgpu/pull/9607). - Fixed missing initialization of other aspects when writing to a single aspect of a multi-aspect texture. By @andyleiserson in [#9626](https://github.com/gfx-rs/wgpu/pull/9626). +- Fix process abort when a `SurfaceTexture` is dropped during panic unwind between `get_current_texture` and `Queue::present`. The acquired texture reference is now released without calling HAL discard. By @hack3rmann in [#9678](https://github.com/gfx-rs/wgpu/pull/9678). #### naga diff --git a/player/src/lib.rs b/player/src/lib.rs index 296370b25c6..bb26175a6fa 100644 --- a/player/src/lib.rs +++ b/player/src/lib.rs @@ -126,7 +126,8 @@ impl Player { } Action::ConfigureSurface { .. } | Action::Present(_) - | Action::DiscardSurfaceTexture(_) => { + | Action::DiscardSurfaceTexture(_) + | Action::ReleaseSurfaceTexture(_) => { panic!("Unexpected Surface action: winit feature is not enabled") } Action::CreateBuffer(id, desc) => { diff --git a/wgpu-core/src/device/trace.rs b/wgpu-core/src/device/trace.rs index 6d56c94abda..3ed8139e715 100644 --- a/wgpu-core/src/device/trace.rs +++ b/wgpu-core/src/device/trace.rs @@ -154,6 +154,7 @@ pub enum Action<'a, R: ReferenceType> { }, Present(R::Surface), DiscardSurfaceTexture(R::Surface), + ReleaseSurfaceTexture(R::Surface), CreateBindGroupLayout( PointerId, crate::binding_model::BindGroupLayoutDescriptor<'a>, diff --git a/wgpu-core/src/device/trace/record.rs b/wgpu-core/src/device/trace/record.rs index 19beefa8f61..46e4d56ebcc 100644 --- a/wgpu-core/src/device/trace/record.rs +++ b/wgpu-core/src/device/trace/record.rs @@ -905,6 +905,7 @@ fn action_to_owned(action: Action<'_, PointerReferences>) -> Action<'static, Poi A::GetSurfaceTexture { id, parent } => A::GetSurfaceTexture { id, parent }, A::Present(surface) => A::Present(surface), A::DiscardSurfaceTexture(surface) => A::DiscardSurfaceTexture(surface), + A::ReleaseSurfaceTexture(surface) => A::ReleaseSurfaceTexture(surface), A::DropBindGroupLayout(layout) => A::DropBindGroupLayout(layout), A::GetRenderPipelineBindGroupLayout { id, diff --git a/wgpu-core/src/present.rs b/wgpu-core/src/present.rs index b7f92b58435..4303b0c14e2 100644 --- a/wgpu-core/src/present.rs +++ b/wgpu-core/src/present.rs @@ -401,6 +401,28 @@ impl Surface { Ok(()) } + + /// Like `discard`, drops the inner texture reference, but skips the + /// HAL `discard_texture` call. Safe to call during unwinding + pub fn release(&self) -> Result<(), SurfaceError> { + profiling::scope!("Surface::release"); + + let mut presentation = self.presentation.lock(); + let Some(present) = presentation.as_mut() else { + return Err(SurfaceError::NotConfigured); + }; + + // `texture` is dropped here, decrementing the refcount of + // Arc. If this was the last Arc, the Texture + // is freed, which drops NativeSurfaceTextureMetadata and + // its Arc. + _ = present + .acquired_texture + .take() + .ok_or(SurfaceError::NothingToPresent)?; + + Ok(()) + } } impl Global { @@ -463,4 +485,17 @@ impl Global { surface.discard() } + + pub fn surface_texture_release(&self, surface_id: id::SurfaceId) -> Result<(), SurfaceError> { + let surface = self.surfaces.get(surface_id); + + #[cfg(feature = "trace")] + if let Some(present) = surface.presentation.lock().as_ref() { + if let Some(ref mut trace) = *present.device.trace.lock() { + trace.add(Action::ReleaseSurfaceTexture(surface.to_trace())); + } + } + + surface.release() + } } diff --git a/wgpu/src/api/surface_texture.rs b/wgpu/src/api/surface_texture.rs index 5126c10bb85..acb83867726 100644 --- a/wgpu/src/api/surface_texture.rs +++ b/wgpu/src/api/surface_texture.rs @@ -28,8 +28,15 @@ impl SurfaceTexture { impl Drop for SurfaceTexture { fn drop(&mut self) { - if !self.presented && !thread_panicking() { - self.detail.texture_discard(); + if !self.presented { + if thread_panicking() { + // Best effort: release reference to `SwapchainAcquireSemaphore` + // This fixes + // `Trying to destroy a SwapchainAcquireSemaphore that is still in use by a SurfaceTexture` + self.detail.texture_release(); + } else { + self.detail.texture_discard(); + } } } } diff --git a/wgpu/src/backend/webgpu.rs b/wgpu/src/backend/webgpu.rs index f3eb0da7ecb..cbe8e5ee89b 100644 --- a/wgpu/src/backend/webgpu.rs +++ b/wgpu/src/backend/webgpu.rs @@ -4028,6 +4028,10 @@ impl dispatch::SurfaceOutputDetailInterface for WebSurfaceOutputDetail { fn texture_discard(&self) { // Can't really discard the texture on the web. } + + fn texture_release(&self) { + // Can't really discard the texture on the web, so there's no point of releasing too + } } impl Drop for WebSurfaceOutputDetail { fn drop(&mut self) { diff --git a/wgpu/src/backend/wgpu_core.rs b/wgpu/src/backend/wgpu_core.rs index c894c5c1925..385a85fd3bb 100644 --- a/wgpu/src/backend/wgpu_core.rs +++ b/wgpu/src/backend/wgpu_core.rs @@ -4029,6 +4029,16 @@ impl dispatch::SurfaceOutputDetailInterface for CoreSurfaceOutputDetail { } } } + + fn texture_release(&self) { + match self.context.0.surface_texture_release(self.surface_id) { + Ok(_status) => (), + Err(err) => { + self.context + .handle_error_nolabel(&self.error_sink, err, "Surface::release_texture") + } + } + } } impl Drop for CoreSurfaceOutputDetail { fn drop(&mut self) { diff --git a/wgpu/src/dispatch.rs b/wgpu/src/dispatch.rs index 11ab0c9d125..4e17d81c5eb 100644 --- a/wgpu/src/dispatch.rs +++ b/wgpu/src/dispatch.rs @@ -609,6 +609,7 @@ pub trait SurfaceInterface: CommonTraits { pub trait SurfaceOutputDetailInterface: CommonTraits { fn texture_discard(&self); + fn texture_release(&self); } pub trait QueueWriteBufferInterface: CommonTraits {