From 1682703b5cb09095fcfb23fbf5f92827e99dda2a Mon Sep 17 00:00:00 2001 From: John Nunley Date: Fri, 26 Apr 2024 09:28:10 -0700 Subject: [PATCH] bugfix(win32): Only return win handle on OK thread On Windows, it is generally unsafe to use the HWND outside of the thread that it originates from. In reality, the HWND is an index into a thread-local table, so using it outside of the GUI thread can result in another window being used instead, following by code unsoundness. This is why the WindowHandle type is !Send. However, Window is Send and Sync, which means we have to account for this. Thus far the best solution seems to be to check if we are not in the GUI thread. If we aren't, refuse the return the window handle. For users who want to ensure the safety themselves, the unsafe API was added. Signed-off-by: John Nunley --- src/changelog/unreleased.md | 2 + src/platform/windows.rs | 150 ++++++++++++++++++++++++++++ src/platform_impl/windows/window.rs | 18 +++- 3 files changed, 169 insertions(+), 1 deletion(-) diff --git a/src/changelog/unreleased.md b/src/changelog/unreleased.md index 3eda9c37cf..28484b354c 100644 --- a/src/changelog/unreleased.md +++ b/src/changelog/unreleased.md @@ -64,6 +64,7 @@ changelog entry. - On Windows, add `with_title_text_color`, and `with_corner_preference` on `WindowAttributesExtWindows`. - On Windows, implement resize increments. +- On Windows, add `AnyThread` API to access window handle off the main thread. ### Changed @@ -256,3 +257,4 @@ changelog entry. - On macOS, fix sequence of mouse events being out of order when dragging on the trackpad. - On Wayland, fix decoration glitch on close with some compositors. - On Android, fix a regression introduced in #2748 to allow volume key events to be received again. +- On Windows, don't return a valid window handle outside of the GUI thread. diff --git a/src/platform/windows.rs b/src/platform/windows.rs index 16ff8eab9b..bce4637ba6 100644 --- a/src/platform/windows.rs +++ b/src/platform/windows.rs @@ -2,6 +2,7 @@ //! //! The supported OS version is Windows 7 or higher, though Windows 10 is //! tested regularly. +use std::borrow::Borrow; use std::ffi::c_void; use std::path::Path; @@ -104,6 +105,60 @@ pub enum CornerPreference { RoundSmall = 3, } +/// A wrapper around a [`Window`] that ignores thread-specific window handle limitations. +/// +/// See [`WindowBorrowExtWindows::any_thread`] for more information. +#[derive(Debug)] +pub struct AnyThread(W); + +impl> AnyThread { + /// Get a reference to the inner window. + #[inline] + pub fn get_ref(&self) -> &Window { + self.0.borrow() + } + + /// Get a reference to the inner object. + #[inline] + pub fn inner(&self) -> &W { + &self.0 + } + + /// Unwrap and get the inner window. + #[inline] + pub fn into_inner(self) -> W { + self.0 + } +} + +impl> AsRef for AnyThread { + fn as_ref(&self) -> &Window { + self.get_ref() + } +} + +impl> Borrow for AnyThread { + fn borrow(&self) -> &Window { + self.get_ref() + } +} + +impl> std::ops::Deref for AnyThread { + type Target = Window; + + fn deref(&self) -> &Self::Target { + self.get_ref() + } +} + +#[cfg(feature = "rwh_06")] +impl> rwh_06::HasWindowHandle for AnyThread { + fn window_handle(&self) -> Result, rwh_06::HandleError> { + // SAFETY: The top level user has asserted this is only used safely. + unsafe { self.get_ref().window_handle_any_thread() } + } +} + /// Additional methods on `EventLoop` that are specific to Windows. pub trait EventLoopBuilderExtWindows { /// Whether to allow the event loop to be created off of the main thread. @@ -247,6 +302,60 @@ pub trait WindowExtWindows { /// /// Supported starting with Windows 11 Build 22000. fn set_corner_preference(&self, preference: CornerPreference); + + /// Get the raw window handle for this [`Window`] without checking for thread affinity. + /// + /// Window handles in Win32 have a property called "thread affinity" that ties them to their + /// origin thread. Some operations can only happen on the window's origin thread, while others + /// can be called from any thread. For example, [`SetWindowSubclass`] is not thread safe while + /// [`GetDC`] is thread safe. + /// + /// In Rust terms, the window handle is `Send` sometimes but `!Send` other times. + /// + /// Therefore, in order to avoid confusing threading errors, [`Window`] only returns the + /// window handle when the [`window_handle`] function is called from the thread that created + /// the window. In other cases, it returns an [`Unavailable`] error. + /// + /// However in some cases you may already know that you are using the window handle for + /// operations that are guaranteed to be thread-safe. In which case this function aims + /// to provide an escape hatch so these functions are still accessible from other threads. + /// + /// # Safety + /// + /// It is the responsibility of the user to only pass the window handle into thread-safe + /// Win32 APIs. + /// + /// [`SetWindowSubclass`]: https://learn.microsoft.com/en-us/windows/win32/api/commctrl/nf-commctrl-setwindowsubclass + /// [`GetDC`]: https://learn.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-getdc + /// [`Window`]: crate::window::Window + /// [`window_handle`]: https://docs.rs/raw-window-handle/latest/raw_window_handle/trait.HasWindowHandle.html#tymethod.window_handle + /// [`Unavailable`]: https://docs.rs/raw-window-handle/latest/raw_window_handle/enum.HandleError.html#variant.Unavailable + /// + /// ## Example + /// + /// ```no_run + /// # use winit::window::Window; + /// # fn scope(window: Window) { + /// use std::thread; + /// use winit::platform::windows::WindowExtWindows; + /// use winit::raw_window_handle::HasWindowHandle; + /// + /// // We can get the window handle on the current thread. + /// let handle = window.window_handle().unwrap(); + /// + /// // However, on another thread, we can't! + /// thread::spawn(move || { + /// assert!(window.window_handle().is_err()); + /// + /// // We can use this function as an escape hatch. + /// let handle = unsafe { window.window_handle_any_thread().unwrap() }; + /// }); + /// # } + /// ``` + #[cfg(feature = "rwh_06")] + unsafe fn window_handle_any_thread( + &self, + ) -> Result, rwh_06::HandleError>; } impl WindowExtWindows for Window { @@ -297,8 +406,49 @@ impl WindowExtWindows for Window { fn set_corner_preference(&self, preference: CornerPreference) { self.window.set_corner_preference(preference) } + + #[cfg(feature = "rwh_06")] + unsafe fn window_handle_any_thread( + &self, + ) -> Result, rwh_06::HandleError> { + unsafe { + let handle = self.window.rwh_06_no_thread_check()?; + + // SAFETY: The handle is valid in this context. + Ok(rwh_06::WindowHandle::borrow_raw(handle)) + } + } } +/// Additional methods for anything that dereference to [`Window`]. +/// +/// [`Window`]: crate::window::Window +pub trait WindowBorrowExtWindows: Borrow + Sized { + /// Create an object that allows accessing the inner window handle in a thread-unsafe way. + /// + /// It is possible to call [`window_handle_any_thread`] to get around Windows's thread + /// affinity limitations. However, it may be desired to pass the [`Window`] into something + /// that requires the [`HasWindowHandle`] trait, while ignoring thread affinity limitations. + /// + /// This function wraps anything that implements `Borrow` into a structure that + /// uses the inner window handle as a mean of implementing [`HasWindowHandle`]. It wraps + /// `Window`, `&Window`, `Arc`, and other reference types. + /// + /// # Safety + /// + /// It is the responsibility of the user to only pass the window handle into thread-safe + /// Win32 APIs. + /// + /// [`window_handle_any_thread`]: WindowExtWindows::window_handle_any_thread + /// [`Window`]: crate::window::Window + /// [`HasWindowHandle`]: rwh_06::HasWindowHandle + unsafe fn any_thread(self) -> AnyThread { + AnyThread(self) + } +} + +impl + Sized> WindowBorrowExtWindows for W {} + /// Additional methods on `WindowAttributes` that are specific to Windows. #[allow(rustdoc::broken_intra_doc_links)] pub trait WindowAttributesExtWindows { diff --git a/src/platform_impl/windows/window.rs b/src/platform_impl/windows/window.rs index 01589bef71..23fec32b34 100644 --- a/src/platform_impl/windows/window.rs +++ b/src/platform_impl/windows/window.rs @@ -365,7 +365,9 @@ impl Window { #[cfg(feature = "rwh_06")] #[inline] - pub fn raw_window_handle_rwh_06(&self) -> Result { + pub unsafe fn rwh_06_no_thread_check( + &self, + ) -> Result { let mut window_handle = rwh_06::Win32WindowHandle::new(unsafe { // SAFETY: Handle will never be zero. std::num::NonZeroIsize::new_unchecked(self.window) @@ -375,6 +377,20 @@ impl Window { Ok(rwh_06::RawWindowHandle::Win32(window_handle)) } + #[cfg(feature = "rwh_06")] + #[inline] + pub fn raw_window_handle_rwh_06(&self) -> Result { + // TODO: Write a test once integration framework is ready to ensure that it holds. + // If we aren't in the GUI thread, we can't return the window. + if !self.thread_executor.in_event_loop_thread() { + tracing::error!("tried to access window handle outside of the main thread"); + return Err(rwh_06::HandleError::Unavailable); + } + + // SAFETY: We are on the correct thread. + unsafe { self.rwh_06_no_thread_check() } + } + #[cfg(feature = "rwh_06")] #[inline] pub fn raw_display_handle_rwh_06(