Skip to content

Commit 1682703

Browse files
authored
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 <dev@notgull.net>
1 parent bdd80c8 commit 1682703

File tree

3 files changed

+169
-1
lines changed

3 files changed

+169
-1
lines changed

src/changelog/unreleased.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ changelog entry.
6464
- On Windows, add `with_title_text_color`, and `with_corner_preference` on
6565
`WindowAttributesExtWindows`.
6666
- On Windows, implement resize increments.
67+
- On Windows, add `AnyThread` API to access window handle off the main thread.
6768

6869
### Changed
6970

@@ -256,3 +257,4 @@ changelog entry.
256257
- On macOS, fix sequence of mouse events being out of order when dragging on the trackpad.
257258
- On Wayland, fix decoration glitch on close with some compositors.
258259
- On Android, fix a regression introduced in #2748 to allow volume key events to be received again.
260+
- On Windows, don't return a valid window handle outside of the GUI thread.

src/platform/windows.rs

Lines changed: 150 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
//!
33
//! The supported OS version is Windows 7 or higher, though Windows 10 is
44
//! tested regularly.
5+
use std::borrow::Borrow;
56
use std::ffi::c_void;
67
use std::path::Path;
78

@@ -104,6 +105,60 @@ pub enum CornerPreference {
104105
RoundSmall = 3,
105106
}
106107

108+
/// A wrapper around a [`Window`] that ignores thread-specific window handle limitations.
109+
///
110+
/// See [`WindowBorrowExtWindows::any_thread`] for more information.
111+
#[derive(Debug)]
112+
pub struct AnyThread<W>(W);
113+
114+
impl<W: Borrow<Window>> AnyThread<W> {
115+
/// Get a reference to the inner window.
116+
#[inline]
117+
pub fn get_ref(&self) -> &Window {
118+
self.0.borrow()
119+
}
120+
121+
/// Get a reference to the inner object.
122+
#[inline]
123+
pub fn inner(&self) -> &W {
124+
&self.0
125+
}
126+
127+
/// Unwrap and get the inner window.
128+
#[inline]
129+
pub fn into_inner(self) -> W {
130+
self.0
131+
}
132+
}
133+
134+
impl<W: Borrow<Window>> AsRef<Window> for AnyThread<W> {
135+
fn as_ref(&self) -> &Window {
136+
self.get_ref()
137+
}
138+
}
139+
140+
impl<W: Borrow<Window>> Borrow<Window> for AnyThread<W> {
141+
fn borrow(&self) -> &Window {
142+
self.get_ref()
143+
}
144+
}
145+
146+
impl<W: Borrow<Window>> std::ops::Deref for AnyThread<W> {
147+
type Target = Window;
148+
149+
fn deref(&self) -> &Self::Target {
150+
self.get_ref()
151+
}
152+
}
153+
154+
#[cfg(feature = "rwh_06")]
155+
impl<W: Borrow<Window>> rwh_06::HasWindowHandle for AnyThread<W> {
156+
fn window_handle(&self) -> Result<rwh_06::WindowHandle<'_>, rwh_06::HandleError> {
157+
// SAFETY: The top level user has asserted this is only used safely.
158+
unsafe { self.get_ref().window_handle_any_thread() }
159+
}
160+
}
161+
107162
/// Additional methods on `EventLoop` that are specific to Windows.
108163
pub trait EventLoopBuilderExtWindows {
109164
/// Whether to allow the event loop to be created off of the main thread.
@@ -247,6 +302,60 @@ pub trait WindowExtWindows {
247302
///
248303
/// Supported starting with Windows 11 Build 22000.
249304
fn set_corner_preference(&self, preference: CornerPreference);
305+
306+
/// Get the raw window handle for this [`Window`] without checking for thread affinity.
307+
///
308+
/// Window handles in Win32 have a property called "thread affinity" that ties them to their
309+
/// origin thread. Some operations can only happen on the window's origin thread, while others
310+
/// can be called from any thread. For example, [`SetWindowSubclass`] is not thread safe while
311+
/// [`GetDC`] is thread safe.
312+
///
313+
/// In Rust terms, the window handle is `Send` sometimes but `!Send` other times.
314+
///
315+
/// Therefore, in order to avoid confusing threading errors, [`Window`] only returns the
316+
/// window handle when the [`window_handle`] function is called from the thread that created
317+
/// the window. In other cases, it returns an [`Unavailable`] error.
318+
///
319+
/// However in some cases you may already know that you are using the window handle for
320+
/// operations that are guaranteed to be thread-safe. In which case this function aims
321+
/// to provide an escape hatch so these functions are still accessible from other threads.
322+
///
323+
/// # Safety
324+
///
325+
/// It is the responsibility of the user to only pass the window handle into thread-safe
326+
/// Win32 APIs.
327+
///
328+
/// [`SetWindowSubclass`]: https://learn.microsoft.com/en-us/windows/win32/api/commctrl/nf-commctrl-setwindowsubclass
329+
/// [`GetDC`]: https://learn.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-getdc
330+
/// [`Window`]: crate::window::Window
331+
/// [`window_handle`]: https://docs.rs/raw-window-handle/latest/raw_window_handle/trait.HasWindowHandle.html#tymethod.window_handle
332+
/// [`Unavailable`]: https://docs.rs/raw-window-handle/latest/raw_window_handle/enum.HandleError.html#variant.Unavailable
333+
///
334+
/// ## Example
335+
///
336+
/// ```no_run
337+
/// # use winit::window::Window;
338+
/// # fn scope(window: Window) {
339+
/// use std::thread;
340+
/// use winit::platform::windows::WindowExtWindows;
341+
/// use winit::raw_window_handle::HasWindowHandle;
342+
///
343+
/// // We can get the window handle on the current thread.
344+
/// let handle = window.window_handle().unwrap();
345+
///
346+
/// // However, on another thread, we can't!
347+
/// thread::spawn(move || {
348+
/// assert!(window.window_handle().is_err());
349+
///
350+
/// // We can use this function as an escape hatch.
351+
/// let handle = unsafe { window.window_handle_any_thread().unwrap() };
352+
/// });
353+
/// # }
354+
/// ```
355+
#[cfg(feature = "rwh_06")]
356+
unsafe fn window_handle_any_thread(
357+
&self,
358+
) -> Result<rwh_06::WindowHandle<'_>, rwh_06::HandleError>;
250359
}
251360

252361
impl WindowExtWindows for Window {
@@ -297,8 +406,49 @@ impl WindowExtWindows for Window {
297406
fn set_corner_preference(&self, preference: CornerPreference) {
298407
self.window.set_corner_preference(preference)
299408
}
409+
410+
#[cfg(feature = "rwh_06")]
411+
unsafe fn window_handle_any_thread(
412+
&self,
413+
) -> Result<rwh_06::WindowHandle<'_>, rwh_06::HandleError> {
414+
unsafe {
415+
let handle = self.window.rwh_06_no_thread_check()?;
416+
417+
// SAFETY: The handle is valid in this context.
418+
Ok(rwh_06::WindowHandle::borrow_raw(handle))
419+
}
420+
}
300421
}
301422

423+
/// Additional methods for anything that dereference to [`Window`].
424+
///
425+
/// [`Window`]: crate::window::Window
426+
pub trait WindowBorrowExtWindows: Borrow<Window> + Sized {
427+
/// Create an object that allows accessing the inner window handle in a thread-unsafe way.
428+
///
429+
/// It is possible to call [`window_handle_any_thread`] to get around Windows's thread
430+
/// affinity limitations. However, it may be desired to pass the [`Window`] into something
431+
/// that requires the [`HasWindowHandle`] trait, while ignoring thread affinity limitations.
432+
///
433+
/// This function wraps anything that implements `Borrow<Window>` into a structure that
434+
/// uses the inner window handle as a mean of implementing [`HasWindowHandle`]. It wraps
435+
/// `Window`, `&Window`, `Arc<Window>`, and other reference types.
436+
///
437+
/// # Safety
438+
///
439+
/// It is the responsibility of the user to only pass the window handle into thread-safe
440+
/// Win32 APIs.
441+
///
442+
/// [`window_handle_any_thread`]: WindowExtWindows::window_handle_any_thread
443+
/// [`Window`]: crate::window::Window
444+
/// [`HasWindowHandle`]: rwh_06::HasWindowHandle
445+
unsafe fn any_thread(self) -> AnyThread<Self> {
446+
AnyThread(self)
447+
}
448+
}
449+
450+
impl<W: Borrow<Window> + Sized> WindowBorrowExtWindows for W {}
451+
302452
/// Additional methods on `WindowAttributes` that are specific to Windows.
303453
#[allow(rustdoc::broken_intra_doc_links)]
304454
pub trait WindowAttributesExtWindows {

src/platform_impl/windows/window.rs

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -365,7 +365,9 @@ impl Window {
365365

366366
#[cfg(feature = "rwh_06")]
367367
#[inline]
368-
pub fn raw_window_handle_rwh_06(&self) -> Result<rwh_06::RawWindowHandle, rwh_06::HandleError> {
368+
pub unsafe fn rwh_06_no_thread_check(
369+
&self,
370+
) -> Result<rwh_06::RawWindowHandle, rwh_06::HandleError> {
369371
let mut window_handle = rwh_06::Win32WindowHandle::new(unsafe {
370372
// SAFETY: Handle will never be zero.
371373
std::num::NonZeroIsize::new_unchecked(self.window)
@@ -375,6 +377,20 @@ impl Window {
375377
Ok(rwh_06::RawWindowHandle::Win32(window_handle))
376378
}
377379

380+
#[cfg(feature = "rwh_06")]
381+
#[inline]
382+
pub fn raw_window_handle_rwh_06(&self) -> Result<rwh_06::RawWindowHandle, rwh_06::HandleError> {
383+
// TODO: Write a test once integration framework is ready to ensure that it holds.
384+
// If we aren't in the GUI thread, we can't return the window.
385+
if !self.thread_executor.in_event_loop_thread() {
386+
tracing::error!("tried to access window handle outside of the main thread");
387+
return Err(rwh_06::HandleError::Unavailable);
388+
}
389+
390+
// SAFETY: We are on the correct thread.
391+
unsafe { self.rwh_06_no_thread_check() }
392+
}
393+
378394
#[cfg(feature = "rwh_06")]
379395
#[inline]
380396
pub fn raw_display_handle_rwh_06(

0 commit comments

Comments
 (0)