Remove unsound SendSyncWrapper (#3303)

CHANGELOG.md

@@ -24,6 +24,7 @@ Unreleased` header.
 - **Breaking:** On Web, macOS and iOS, return `HandleError::Unavailable` when a window handle is not available.
 - **Breaking:** Bump MSRV from `1.65` to `1.70`.
 - On Web, add the ability to toggle calling `Event.preventDefault()` on `Window`.
+- **Breaking:** Remove `WindowAttributes::fullscreen()` and expose as field directly.
 
 # 0.29.6
 

src/lib.rs

@@ -173,18 +173,3 @@ mod platform_impl;
 pub mod window;
 
 pub mod platform;
-
-/// Wrapper for objects which winit will access on the main thread so they are effectively `Send`
-/// and `Sync`, since they always execute on a single thread.
-///
-/// # Safety
-///
-/// Winit can run only one event loop at a time, and the event loop itself is tied to some thread.
-/// The objects could be sent across the threads, but once passed to winit, they execute on the
-/// main thread if the platform demands it. Thus, marking such objects as `Send + Sync` is safe.
-#[doc(hidden)]
-#[derive(Clone, Debug)]
-pub(crate) struct SendSyncWrapper<T>(pub(crate) T);
-
-unsafe impl<T> Send for SendSyncWrapper<T> {}
-unsafe impl<T> Sync for SendSyncWrapper<T> {}

src/platform_impl/ios/view.rs

@@ -475,7 +475,7 @@ impl WinitUIWindow {
 
         this.setRootViewController(Some(view_controller));
 
-        match window_attributes.fullscreen.0.clone().map(Into::into) {
+        match window_attributes.fullscreen.clone().map(Into::into) {
             Some(Fullscreen::Exclusive(ref video_mode)) => {
                 let monitor = video_mode.monitor();
                 let screen = monitor.ui_screen(mtm);

src/platform_impl/ios/window.rs

@@ -367,15 +367,6 @@ impl Inner {
         rwh_06::RawWindowHandle::UiKit(window_handle)
     }
 
-    #[cfg(feature = "rwh_06")]
-    pub fn raw_display_handle_rwh_06(
-        &self,
-    ) -> Result<rwh_06::RawDisplayHandle, rwh_06::HandleError> {
-        Ok(rwh_06::RawDisplayHandle::UiKit(
-            rwh_06::UiKitDisplayHandle::new(),
-        ))
-    }
-
     pub fn theme(&self) -> Option<Theme> {
         warn!("`Window::theme` is ignored on iOS");
         None
@@ -424,7 +415,7 @@ impl Window {
         // TODO: transparency, visible
 
         let main_screen = UIScreen::main(mtm);
-        let fullscreen = window_attributes.fullscreen.0.clone().map(Into::into);
+        let fullscreen = window_attributes.fullscreen.clone().map(Into::into);
         let screen = match fullscreen {
             Some(Fullscreen::Exclusive(ref video_mode)) => video_mode.monitor.ui_screen(mtm),
             Some(Fullscreen::Borderless(Some(ref monitor))) => monitor.ui_screen(mtm),
@@ -532,6 +523,16 @@ impl Window {
             Err(rwh_06::HandleError::Unavailable)
         }
     }
+
+    #[cfg(feature = "rwh_06")]
+    #[inline]
+    pub(crate) fn raw_display_handle_rwh_06(
+        &self,
+    ) -> Result<rwh_06::RawDisplayHandle, rwh_06::HandleError> {
+        Ok(rwh_06::RawDisplayHandle::UiKit(
+            rwh_06::UiKitDisplayHandle::new(),
+        ))
+    }
 }
 
 // WindowExtIOS

src/platform_impl/linux/wayland/window/mod.rs

@@ -152,7 +152,7 @@ impl Window {
         window_state.set_resizable(attributes.resizable);
 
         // Set startup mode.
-        match attributes.fullscreen.0.map(Into::into) {
+        match attributes.fullscreen.map(Into::into) {
             Some(Fullscreen::Exclusive(_)) => {
                 warn!("`Fullscreen::Exclusive` is ignored on Wayland");
             }

src/platform_impl/linux/x11/window.rs

@@ -159,7 +159,7 @@ impl UnownedWindow {
         let xconn = &event_loop.xconn;
         let atoms = xconn.atoms();
         #[cfg(feature = "rwh_06")]
-        let root = match window_attrs.parent_window.0 {
+        let root = match window_attrs.parent_window.as_ref().map(|handle| handle.0) {
             Some(rwh_06::RawWindowHandle::Xlib(handle)) => handle.window as xproto::Window,
             Some(rwh_06::RawWindowHandle::Xcb(handle)) => handle.window.get(),
             Some(raw) => unreachable!("Invalid raw window handle {raw:?} on X11"),
@@ -557,10 +557,10 @@ impl UnownedWindow {
             if window_attrs.maximized {
                 leap!(window.set_maximized_inner(window_attrs.maximized)).ignore_error();
             }
-            if window_attrs.fullscreen.0.is_some() {
+            if window_attrs.fullscreen.is_some() {
                 if let Some(flusher) =
                     leap!(window
-                        .set_fullscreen_inner(window_attrs.fullscreen.0.clone().map(Into::into)))
+                        .set_fullscreen_inner(window_attrs.fullscreen.clone().map(Into::into)))
                 {
                     flusher.ignore_error()
                 }

src/platform_impl/macos/monitor.rs

@@ -60,6 +60,7 @@ impl std::fmt::Debug for VideoMode {
 pub struct NativeDisplayMode(pub ffi::CGDisplayModeRef);
 
 unsafe impl Send for NativeDisplayMode {}
+unsafe impl Sync for NativeDisplayMode {}
 
 impl Drop for NativeDisplayMode {
     fn drop(&mut self) {

src/platform_impl/macos/window.rs

@@ -107,6 +107,16 @@ impl Window {
             Err(rwh_06::HandleError::Unavailable)
         }
     }
+
+    #[cfg(feature = "rwh_06")]
+    #[inline]
+    pub(crate) fn raw_display_handle_rwh_06(
+        &self,
+    ) -> Result<rwh_06::RawDisplayHandle, rwh_06::HandleError> {
+        Ok(rwh_06::RawDisplayHandle::AppKit(
+            rwh_06::AppKitDisplayHandle::new(),
+        ))
+    }
 }
 
 #[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
@@ -282,7 +292,7 @@ impl WinitWindow {
         trace_scope!("WinitWindow::new");
 
         let this = autoreleasepool(|_| {
-            let screen = match attrs.fullscreen.0.clone().map(Into::into) {
+            let screen = match attrs.fullscreen.clone().map(Into::into) {
                 Some(Fullscreen::Borderless(Some(monitor)))
                 | Some(Fullscreen::Exclusive(VideoMode { monitor, .. })) => {
                     monitor.ns_screen(mtm).or_else(|| NSScreen::mainScreen(mtm))
@@ -442,7 +452,7 @@ impl WinitWindow {
         .ok_or_else(|| os_error!(OsError::CreationError("Couldn't create `NSWindow`")))?;
 
         #[cfg(feature = "rwh_06")]
-        match attrs.parent_window.0 {
+        match attrs.parent_window.map(|handle| handle.0) {
             Some(rwh_06::RawWindowHandle::AppKit(handle)) => {
                 // SAFETY: Caller ensures the pointer is valid or NULL
                 // Unwrap is fine, since the pointer comes from `NonNull`.
@@ -520,14 +530,14 @@ impl WinitWindow {
 
         this.set_cursor(attrs.cursor);
 
-        let delegate = WinitWindowDelegate::new(&this, attrs.fullscreen.0.is_some());
+        let delegate = WinitWindowDelegate::new(&this, attrs.fullscreen.is_some());
 
         // XXX Send `Focused(false)` right after creating the window delegate, so we won't
         // obscure the real focused events on the startup.
         delegate.queue_event(WindowEvent::Focused(false));
 
         // Set fullscreen mode after we setup everything
-        this.set_fullscreen(attrs.fullscreen.0.map(Into::into));
+        this.set_fullscreen(attrs.fullscreen.map(Into::into));
 
         // Setting the window as key has to happen *after* we set the fullscreen
         // state, since otherwise we'll briefly see the window at normal size
@@ -1368,16 +1378,6 @@ impl WinitWindow {
         rwh_06::RawWindowHandle::AppKit(window_handle)
     }
 
-    #[cfg(feature = "rwh_06")]
-    #[inline]
-    pub fn raw_display_handle_rwh_06(
-        &self,
-    ) -> Result<rwh_06::RawDisplayHandle, rwh_06::HandleError> {
-        Ok(rwh_06::RawDisplayHandle::AppKit(
-            rwh_06::AppKitDisplayHandle::new(),
-        ))
-    }
-
     fn toggle_style_mask(&self, mask: NSWindowStyleMask, on: bool) {
         let current_style_mask = self.styleMask();
         if on {

src/platform_impl/web/web_sys/canvas.rs

@@ -142,7 +142,7 @@ impl Canvas {
             super::set_canvas_position(&common.document, &common.raw, &common.style, position);
         }
 
-        if attr.fullscreen.0.is_some() {
+        if attr.fullscreen.is_some() {
             fullscreen::request_fullscreen(&document, &canvas);
         }
 

src/platform_impl/web/window.rs

@@ -112,6 +112,16 @@ impl Window {
             })
             .ok_or(rwh_06::HandleError::Unavailable)
     }
+
+    #[cfg(feature = "rwh_06")]
+    #[inline]
+    pub(crate) fn raw_display_handle_rwh_06(
+        &self,
+    ) -> Result<rwh_06::RawDisplayHandle, rwh_06::HandleError> {
+        Ok(rwh_06::RawDisplayHandle::Web(
+            rwh_06::WebDisplayHandle::new(),
+        ))
+    }
 }
 
 impl Inner {
@@ -400,16 +410,6 @@ impl Inner {
         rwh_05::RawDisplayHandle::Web(rwh_05::WebDisplayHandle::empty())
     }
 
-    #[cfg(feature = "rwh_06")]
-    #[inline]
-    pub fn raw_display_handle_rwh_06(
-        &self,
-    ) -> Result<rwh_06::RawDisplayHandle, rwh_06::HandleError> {
-        Ok(rwh_06::RawDisplayHandle::Web(
-            rwh_06::WebDisplayHandle::new(),
-        ))
-    }
-
     #[inline]
     pub fn set_theme(&self, _theme: Option<Theme>) {}
 

src/platform_impl/windows/window.rs

@@ -1233,8 +1233,8 @@ impl<'a, T: 'static> InitData<'a, T> {
 
         win.set_enabled_buttons(attributes.enabled_buttons);
 
-        if attributes.fullscreen.0.is_some() {
+        if attributes.fullscreen.is_some() {
-            win.set_fullscreen(attributes.fullscreen.0.map(Into::into));
+            win.set_fullscreen(attributes.fullscreen.map(Into::into));
             unsafe { force_window_active(win.window) };
         } else {
             let size = attributes
@@ -1320,7 +1320,7 @@ where
     };
 
     #[cfg(feature = "rwh_06")]
-    let parent = match attributes.parent_window.0 {
+    let parent = match attributes.parent_window.as_ref().map(|handle| handle.0) {
         Some(rwh_06::RawWindowHandle::Win32(handle)) => {
             window_flags.set(WindowFlags::CHILD, true);
             if pl_attribs.menu.is_some() {

src/window.rs

@@ -6,7 +6,7 @@ use crate::{
     error::{ExternalError, NotSupportedError, OsError},
     event_loop::EventLoopWindowTarget,
     monitor::{MonitorHandle, VideoMode},
-    platform_impl, SendSyncWrapper,
+    platform_impl,
 };
 
 pub use crate::cursor::{BadImage, Cursor, CustomCursor, CustomCursorBuilder, MAX_CURSOR_SIZE};
@@ -157,8 +157,8 @@ pub struct WindowAttributes {
     pub active: bool,
     pub cursor: Cursor,
     #[cfg(feature = "rwh_06")]
-    pub(crate) parent_window: SendSyncWrapper<Option<rwh_06::RawWindowHandle>>,
+    pub(crate) parent_window: Option<SendSyncRawWindowHandle>,
-    pub(crate) fullscreen: SendSyncWrapper<Option<Fullscreen>>,
+    pub fullscreen: Option<Fullscreen>,
 }
 
 impl Default for WindowAttributes {
@@ -173,7 +173,7 @@ impl Default for WindowAttributes {
             enabled_buttons: WindowButtons::all(),
             title: "winit window".to_owned(),
             maximized: false,
-            fullscreen: SendSyncWrapper(None),
+            fullscreen: None,
             visible: true,
             transparent: false,
             blur: false,
@@ -185,22 +185,32 @@ impl Default for WindowAttributes {
             content_protected: false,
             cursor: Cursor::default(),
             #[cfg(feature = "rwh_06")]
-            parent_window: SendSyncWrapper(None),
+            parent_window: None,
             active: true,
         }
     }
 }
 
+/// Wrapper for [`rwh_06::RawWindowHandle`] for [`WindowAttributes::parent_window`].
+///
+/// # Safety
+///
+/// The user has to account for that when using [`WindowBuilder::with_parent_window()`],
+/// which is `unsafe`.
+#[derive(Debug, Clone)]
+#[cfg(feature = "rwh_06")]
+pub(crate) struct SendSyncRawWindowHandle(pub(crate) rwh_06::RawWindowHandle);
+
+#[cfg(feature = "rwh_06")]
+unsafe impl Send for SendSyncRawWindowHandle {}
+#[cfg(feature = "rwh_06")]
+unsafe impl Sync for SendSyncRawWindowHandle {}
+
 impl WindowAttributes {
     /// Get the parent window stored on the attributes.
     #[cfg(feature = "rwh_06")]
     pub fn parent_window(&self) -> Option<&rwh_06::RawWindowHandle> {
-        self.parent_window.0.as_ref()
+        self.parent_window.as_ref().map(|handle| &handle.0)
-    }
-
-    /// Get `Fullscreen` option stored on the attributes.
-    pub fn fullscreen(&self) -> Option<&Fullscreen> {
-        self.fullscreen.0.as_ref()
     }
 }
 
@@ -321,7 +331,7 @@ impl WindowBuilder {
     /// See [`Window::set_fullscreen`] for details.
     #[inline]
     pub fn with_fullscreen(mut self, fullscreen: Option<Fullscreen>) -> Self {
-        self.window.fullscreen = SendSyncWrapper(fullscreen);
+        self.window.fullscreen = fullscreen;
         self
     }
 
@@ -508,7 +518,7 @@ impl WindowBuilder {
         mut self,
         parent_window: Option<rwh_06::RawWindowHandle>,
     ) -> Self {
-        self.window.parent_window = SendSyncWrapper(parent_window);
+        self.window.parent_window = parent_window.map(SendSyncRawWindowHandle);
         self
     }
 
@@ -1545,21 +1555,30 @@ impl rwh_06::HasWindowHandle for Window {
 #[cfg(feature = "rwh_06")]
 impl rwh_06::HasDisplayHandle for Window {
     fn display_handle(&self) -> Result<rwh_06::DisplayHandle<'_>, rwh_06::HandleError> {
-        let raw = self
+        let raw = self.window.raw_display_handle_rwh_06()?;
-            .window
-            .maybe_wait_on_main(|w| w.raw_display_handle_rwh_06().map(SendSyncWrapper))?
-            .0;
 
-        // SAFETY: The window handle will never be deallocated while the window is alive.
+        // SAFETY: The window handle will never be deallocated while the window is alive,
+        // and the main thread safety requirements are upheld internally by each platform.
         Ok(unsafe { rwh_06::DisplayHandle::borrow_raw(raw) })
     }
 }
 
+/// Wrapper to make objects `Send`.
+///
+/// # Safety
+///
+/// This is not safe! This is only used for `RawWindowHandle`, which only has unsafe getters.
+#[cfg(any(feature = "rwh_05", feature = "rwh_04"))]
+struct UnsafeSendWrapper<T>(T);
+
+#[cfg(any(feature = "rwh_05", feature = "rwh_04"))]
+unsafe impl<T> Send for UnsafeSendWrapper<T> {}
+
 #[cfg(feature = "rwh_05")]
 unsafe impl rwh_05::HasRawWindowHandle for Window {
     fn raw_window_handle(&self) -> rwh_05::RawWindowHandle {
         self.window
-            .maybe_wait_on_main(|w| SendSyncWrapper(w.raw_window_handle_rwh_05()))
+            .maybe_wait_on_main(|w| UnsafeSendWrapper(w.raw_window_handle_rwh_05()))
             .0
     }
 }
@@ -1572,7 +1591,7 @@ unsafe impl rwh_05::HasRawDisplayHandle for Window {
     /// [`EventLoop`]: crate::event_loop::EventLoop
     fn raw_display_handle(&self) -> rwh_05::RawDisplayHandle {
         self.window
-            .maybe_wait_on_main(|w| SendSyncWrapper(w.raw_display_handle_rwh_05()))
+            .maybe_wait_on_main(|w| UnsafeSendWrapper(w.raw_display_handle_rwh_05()))
             .0
     }
 }
@@ -1581,7 +1600,7 @@ unsafe impl rwh_05::HasRawDisplayHandle for Window {
 unsafe impl rwh_04::HasRawWindowHandle for Window {
     fn raw_window_handle(&self) -> rwh_04::RawWindowHandle {
         self.window
-            .maybe_wait_on_main(|w| SendSyncWrapper(w.raw_window_handle_rwh_04()))
+            .maybe_wait_on_main(|w| UnsafeSendWrapper(w.raw_window_handle_rwh_04()))
             .0
     }
 }