diff --git a/CHANGELOG.md b/CHANGELOG.md index 01ab7dbd5..5dc805e97 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -69,7 +69,7 @@ By @teoxoy [#6134](https://github.com/gfx-rs/wgpu/pull/6134). #### Naga -* Support constant evaluation for `firstLeadingBit` and `firstTrailingBit` numeric built-ins in WGSL. Front-ends that translate to these built-ins also benefit from constant evaluation. By @ErichDonGubler in [#5101](https://github.com/gfx-rs/wgpu/pull/5101). +- Support constant evaluation for `firstLeadingBit` and `firstTrailingBit` numeric built-ins in WGSL. Front-ends that translate to these built-ins also benefit from constant evaluation. By @ErichDonGubler in [#5101](https://github.com/gfx-rs/wgpu/pull/5101). #### Vulkan @@ -91,8 +91,13 @@ By @teoxoy [#6134](https://github.com/gfx-rs/wgpu/pull/6134). - Fix crash when dropping the surface after the device. By @wumpf in [#6052](https://github.com/gfx-rs/wgpu/pull/6052) - Fix error message that is thrown in create_render_pass to no longer say `compute_pass`. By @matthew-wong1 [#6041](https://github.com/gfx-rs/wgpu/pull/6041) +#### GLES / OpenGL + +- Fix GL debug message callbacks not being properly cleaned up (causing UB). By @Imberflur in [#6114](https://github.com/gfx-rs/wgpu/pull/6114) + ### Changes +- `wgpu_hal::gles::Adapter::new_external` now requires the context to be current when dropping the adapter and related objects. By @Imberflur in [#6114](https://github.com/gfx-rs/wgpu/pull/6114). - Reduce the amount of debug and trace logs emitted by wgpu-core and wgpu-hal. By @nical in [#6065](https://github.com/gfx-rs/wgpu/issues/6065) - `Rg11b10Float` is renamed to `Rg11b10UFloat`. By @sagudev in [#6108](https://github.com/gfx-rs/wgpu/pull/6108) diff --git a/wgpu-hal/src/gles/egl.rs b/wgpu-hal/src/gles/egl.rs index 89baa52a6..86f104ba9 100644 --- a/wgpu-hal/src/gles/egl.rs +++ b/wgpu-hal/src/gles/egl.rs @@ -1,8 +1,10 @@ use glow::HasContext; use once_cell::sync::Lazy; -use parking_lot::{Mutex, MutexGuard, RwLock}; +use parking_lot::{MappedMutexGuard, Mutex, MutexGuard, RwLock}; -use std::{collections::HashMap, ffi, os::raw, ptr, rc::Rc, sync::Arc, time::Duration}; +use std::{ + collections::HashMap, ffi, mem::ManuallyDrop, os::raw, ptr, rc::Rc, sync::Arc, time::Duration, +}; /// The amount of time to wait while trying to obtain a lock to the adapter context const CONTEXT_LOCK_TIMEOUT_SECS: u64 = 1; @@ -295,6 +297,7 @@ impl EglContext { .make_current(self.display, self.pbuffer, self.pbuffer, Some(self.raw)) .unwrap(); } + fn unmake_current(&self) { self.instance .make_current(self.display, None, None, None) @@ -305,7 +308,7 @@ impl EglContext { /// A wrapper around a [`glow::Context`] and the required EGL context that uses locking to guarantee /// exclusive access when shared with multiple threads. pub struct AdapterContext { - glow: Mutex, + glow: Mutex>, egl: Option, } @@ -346,6 +349,31 @@ impl AdapterContext { } } +impl Drop for AdapterContext { + fn drop(&mut self) { + struct CurrentGuard<'a>(&'a EglContext); + impl Drop for CurrentGuard<'_> { + fn drop(&mut self) { + self.0.unmake_current(); + } + } + + // Context must be current when dropped. See safety docs on + // `glow::HasContext`. + // + // NOTE: This is only set to `None` by `Adapter::new_external` which + // requires the context to be current when anything that may be holding + // the `Arc` is dropped. + let _guard = self.egl.as_ref().map(|egl| { + egl.make_current(); + CurrentGuard(egl) + }); + let glow = self.glow.get_mut(); + // SAFETY: Field not used after this. + unsafe { ManuallyDrop::drop(glow) }; + } +} + struct EglContextLock<'a> { instance: &'a Arc, display: khronos_egl::Display, @@ -353,7 +381,7 @@ struct EglContextLock<'a> { /// A guard containing a lock to an [`AdapterContext`] pub struct AdapterContextLock<'a> { - glow: MutexGuard<'a, glow::Context>, + glow: MutexGuard<'a, ManuallyDrop>, egl: Option>, } @@ -387,10 +415,12 @@ impl AdapterContext { /// /// > **Note:** Calling this function **will** still lock the [`glow::Context`] which adds an /// > extra safe-guard against accidental concurrent access to the context. - pub unsafe fn get_without_egl_lock(&self) -> MutexGuard { - self.glow + pub unsafe fn get_without_egl_lock(&self) -> MappedMutexGuard { + let guard = self + .glow .try_lock_for(Duration::from_secs(CONTEXT_LOCK_TIMEOUT_SECS)) - .expect("Could not lock adapter context. This is most-likely a deadlock.") + .expect("Could not lock adapter context. This is most-likely a deadlock."); + MutexGuard::map(guard, |glow| &mut **glow) } /// Obtain a lock to the EGL context and get handle to the [`glow::Context`] that can be used to @@ -1052,6 +1082,8 @@ impl crate::Instance for Instance { unsafe { gl.debug_message_callback(super::gl_debug_message_callback) }; } + // Avoid accidental drop when the context is not current. + let gl = ManuallyDrop::new(gl); inner.egl.unmake_current(); unsafe { @@ -1073,13 +1105,15 @@ impl super::Adapter { /// - The underlying OpenGL ES context must be current. /// - The underlying OpenGL ES context must be current when interfacing with any objects returned by /// wgpu-hal from this adapter. + /// - The underlying OpenGL ES context must be current when dropping this adapter and when + /// dropping any objects returned from this adapter. pub unsafe fn new_external( fun: impl FnMut(&str) -> *const ffi::c_void, ) -> Option> { let context = unsafe { glow::Context::from_loader_function(fun) }; unsafe { Self::expose(AdapterContext { - glow: Mutex::new(context), + glow: Mutex::new(ManuallyDrop::new(context)), egl: None, }) } diff --git a/wgpu-hal/src/gles/wgl.rs b/wgpu-hal/src/gles/wgl.rs index 68bedb11d..ea466f877 100644 --- a/wgpu-hal/src/gles/wgl.rs +++ b/wgpu-hal/src/gles/wgl.rs @@ -9,7 +9,7 @@ use raw_window_handle::{RawDisplayHandle, RawWindowHandle}; use std::{ collections::HashSet, ffi::{c_void, CStr, CString}, - mem, + mem::{self, ManuallyDrop}, os::raw::c_int, ptr, sync::{ @@ -134,11 +134,29 @@ unsafe impl Send for WglContext {} unsafe impl Sync for WglContext {} struct Inner { - gl: glow::Context, + gl: ManuallyDrop, device: InstanceDevice, context: WglContext, } +impl Drop for Inner { + fn drop(&mut self) { + struct CurrentGuard<'a>(&'a WglContext); + impl Drop for CurrentGuard<'_> { + fn drop(&mut self) { + self.0.unmake_current().unwrap(); + } + } + + // Context must be current when dropped. See safety docs on + // `glow::HasContext`. + self.context.make_current(self.device.dc).unwrap(); + let _guard = CurrentGuard(&self.context); + // SAFETY: Field not used after this. + unsafe { ManuallyDrop::drop(&mut self.gl) }; + } +} + unsafe impl Send for Inner {} unsafe impl Sync for Inner {} @@ -497,6 +515,8 @@ impl crate::Instance for Instance { unsafe { gl.debug_message_callback(super::gl_debug_message_callback) }; } + // Avoid accidental drop when the context is not current. + let gl = ManuallyDrop::new(gl); context.unmake_current().map_err(|e| { crate::InstanceError::with_source( String::from("unable to unset the current WGL context"),