From e79d4ce93d5c01dc29d02d47178d12dda8254e89 Mon Sep 17 00:00:00 2001 From: daxpedda Date: Sun, 24 Dec 2023 15:55:19 +0100 Subject: [PATCH] Remove unsound `SendSyncWrapper` --- CHANGELOG.md | 1 + src/lib.rs | 15 -- src/platform/web.rs | 3 +- src/platform_impl/ios/view.rs | 2 +- src/platform_impl/ios/window.rs | 21 +-- src/platform_impl/linux/wayland/window/mod.rs | 2 +- src/platform_impl/linux/x11/window.rs | 6 +- src/platform_impl/macos/monitor.rs | 10 +- src/platform_impl/macos/window.rs | 37 ++-- src/platform_impl/web/async/dispatcher.rs | 5 +- src/platform_impl/web/async/waker.rs | 6 +- src/platform_impl/web/async/wrapper.rs | 54 ++---- src/platform_impl/web/cursor.rs | 159 ++++++------------ src/platform_impl/web/event_loop/runner.rs | 10 +- src/platform_impl/web/main_thread.rs | 96 +++++++++++ src/platform_impl/web/mod.rs | 1 + src/platform_impl/web/web_sys/canvas.rs | 12 +- src/platform_impl/web/window.rs | 55 ++++-- src/platform_impl/windows/window.rs | 6 +- src/window.rs | 60 ++++--- 20 files changed, 313 insertions(+), 248 deletions(-) create mode 100644 src/platform_impl/web/main_thread.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index d2220943167..0ac32af6fa8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ Unreleased` header. - **Breaking:** On Web, return `RawWindowHandle::WebCanvas` instead of `RawWindowHandle::Web`. - **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`. +- **Breaking:** Remove `WindowAttributes::fullscreen()` and expose as field directly. # 0.29.5 diff --git a/src/lib.rs b/src/lib.rs index ce26ef0e9da..1f9390658bb 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -182,18 +182,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(pub(crate) T); - -unsafe impl Send for SendSyncWrapper {} -unsafe impl Sync for SendSyncWrapper {} diff --git a/src/platform/web.rs b/src/platform/web.rs index 2d79914924b..8aab93bfe05 100644 --- a/src/platform/web.rs +++ b/src/platform/web.rs @@ -34,7 +34,6 @@ use crate::event_loop::EventLoopWindowTarget; use crate::platform_impl::PlatformCustomCursorBuilder; use crate::window::CustomCursor; use crate::window::{Window, WindowBuilder}; -use crate::SendSyncWrapper; use web_sys::HtmlCanvasElement; @@ -85,7 +84,7 @@ pub trait WindowBuilderExtWebSys { impl WindowBuilderExtWebSys for WindowBuilder { fn with_canvas(mut self, canvas: Option) -> Self { - self.platform_specific.canvas = SendSyncWrapper(canvas); + self.platform_specific.set_canvas(canvas); self } diff --git a/src/platform_impl/ios/view.rs b/src/platform_impl/ios/view.rs index 9b456e6b325..5a93ad781cc 100644 --- a/src/platform_impl/ios/view.rs +++ b/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); diff --git a/src/platform_impl/ios/window.rs b/src/platform_impl/ios/window.rs index e34801696ed..b0f5a2a9d4d 100644 --- a/src/platform_impl/ios/window.rs +++ b/src/platform_impl/ios/window.rs @@ -369,15 +369,6 @@ impl Inner { rwh_06::RawWindowHandle::UiKit(window_handle) } - #[cfg(feature = "rwh_06")] - pub fn raw_display_handle_rwh_06( - &self, - ) -> Result { - Ok(rwh_06::RawDisplayHandle::UiKit( - rwh_06::UiKitDisplayHandle::new(), - )) - } - pub fn theme(&self) -> Option { warn!("`Window::theme` is ignored on iOS"); None @@ -426,7 +417,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), @@ -534,6 +525,16 @@ impl Window { Err(rwh_06::HandleError::Unavailable) } } + + #[cfg(feature = "rwh_06")] + #[inline] + pub(crate) fn raw_display_handle_rwh_06( + &self, + ) -> Result { + Ok(rwh_06::RawDisplayHandle::UiKit( + rwh_06::UiKitDisplayHandle::new(), + )) + } } // WindowExtIOS diff --git a/src/platform_impl/linux/wayland/window/mod.rs b/src/platform_impl/linux/wayland/window/mod.rs index 74ff1baf116..6b450b76bb8 100644 --- a/src/platform_impl/linux/wayland/window/mod.rs +++ b/src/platform_impl/linux/wayland/window/mod.rs @@ -150,7 +150,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"); } diff --git a/src/platform_impl/linux/x11/window.rs b/src/platform_impl/linux/x11/window.rs index b7ea8e1a12e..68a36b40735 100644 --- a/src/platform_impl/linux/x11/window.rs +++ b/src/platform_impl/linux/x11/window.rs @@ -158,7 +158,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"), @@ -556,10 +556,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() } diff --git a/src/platform_impl/macos/monitor.rs b/src/platform_impl/macos/monitor.rs index cabbc24080b..daaa28291b2 100644 --- a/src/platform_impl/macos/monitor.rs +++ b/src/platform_impl/macos/monitor.rs @@ -1,6 +1,10 @@ #![allow(clippy::unnecessary_cast)] -use std::{collections::VecDeque, fmt}; +use std::{ + collections::VecDeque, + fmt, + sync::{Arc, Mutex}, +}; use core_foundation::{ array::{CFArrayGetCount, CFArrayGetValueAtIndex}, @@ -23,7 +27,7 @@ pub struct VideoMode { bit_depth: u16, refresh_rate_millihertz: u32, pub(crate) monitor: MonitorHandle, - pub(crate) native_mode: NativeDisplayMode, + pub(crate) native_mode: Arc>, } impl PartialEq for VideoMode { @@ -290,7 +294,7 @@ impl MonitorHandle { refresh_rate_millihertz, bit_depth, monitor: monitor.clone(), - native_mode: NativeDisplayMode(mode), + native_mode: Arc::new(Mutex::new(NativeDisplayMode(mode))), } }) } diff --git a/src/platform_impl/macos/window.rs b/src/platform_impl/macos/window.rs index aac58b91847..0d8c09f305a 100644 --- a/src/platform_impl/macos/window.rs +++ b/src/platform_impl/macos/window.rs @@ -3,6 +3,7 @@ use std::collections::VecDeque; use std::f64; use std::ops; +use std::sync::PoisonError; use std::sync::{Mutex, MutexGuard}; use crate::{ @@ -105,6 +106,20 @@ impl Window { Err(rwh_06::HandleError::Unavailable) } } + + #[cfg(feature = "rwh_06")] + #[inline] + pub(crate) fn raw_display_handle_rwh_06( + &self, + ) -> Result { + struct UnsafeSendWrapper(T); + + unsafe impl Send for UnsafeSendWrapper {} + + Ok(self + .maybe_wait_on_main(|w| UnsafeSendWrapper(w.raw_display_handle_rwh_06())) + .0) + } } #[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] @@ -280,7 +295,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)) @@ -440,7 +455,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`. @@ -516,14 +531,14 @@ impl WinitWindow { } } - 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 @@ -1088,7 +1103,11 @@ impl WinitWindow { unsafe { let result = ffi::CGDisplaySetDisplayMode( display_id, - video_mode.native_mode.0, + video_mode + .native_mode + .lock() + .unwrap_or_else(PoisonError::into_inner) + .0, std::ptr::null(), ); assert!(result == ffi::kCGErrorSuccess, "failed to set video mode"); @@ -1363,12 +1382,8 @@ impl WinitWindow { #[cfg(feature = "rwh_06")] #[inline] - pub fn raw_display_handle_rwh_06( - &self, - ) -> Result { - Ok(rwh_06::RawDisplayHandle::AppKit( - rwh_06::AppKitDisplayHandle::new(), - )) + pub fn raw_display_handle_rwh_06(&self) -> rwh_06::RawDisplayHandle { + rwh_06::RawDisplayHandle::AppKit(rwh_06::AppKitDisplayHandle::new()) } fn toggle_style_mask(&self, mask: NSWindowStyleMask, on: bool) { diff --git a/src/platform_impl/web/async/dispatcher.rs b/src/platform_impl/web/async/dispatcher.rs index 5fb63d54567..3b1e6d78453 100644 --- a/src/platform_impl/web/async/dispatcher.rs +++ b/src/platform_impl/web/async/dispatcher.rs @@ -1,3 +1,5 @@ +use super::super::main_thread::MainThreadMarker; + use super::{channel, AsyncReceiver, AsyncSender, Wrapper}; use std::{ cell::Ref, @@ -10,10 +12,11 @@ struct Closure(Box); impl Dispatcher { #[track_caller] - pub fn new(value: T) -> Option<(Self, DispatchRunner)> { + pub fn new(main_thread: MainThreadMarker, value: T) -> Option<(Self, DispatchRunner)> { let (sender, receiver) = channel::>(); Wrapper::new( + main_thread, value, |value, Closure(closure)| { // SAFETY: The given `Closure` here isn't really `'static`, so we shouldn't do anything diff --git a/src/platform_impl/web/async/waker.rs b/src/platform_impl/web/async/waker.rs index 2c27c474c92..79062d84569 100644 --- a/src/platform_impl/web/async/waker.rs +++ b/src/platform_impl/web/async/waker.rs @@ -1,3 +1,4 @@ +use super::super::main_thread::MainThreadMarker; use super::Wrapper; use atomic_waker::AtomicWaker; use std::future; @@ -19,7 +20,7 @@ struct Sender(Arc); impl WakerSpawner { #[track_caller] - pub fn new(value: T, handler: fn(&T, usize)) -> Option { + pub fn new(main_thread: MainThreadMarker, value: T, handler: fn(&T, usize)) -> Option { let inner = Arc::new(Inner { counter: AtomicUsize::new(0), waker: AtomicWaker::new(), @@ -31,6 +32,7 @@ impl WakerSpawner { let sender = Sender(Arc::clone(&inner)); let wrapper = Wrapper::new( + main_thread, handler, |handler, count| { let handler = handler.borrow(); @@ -86,7 +88,7 @@ impl WakerSpawner { pub fn fetch(&self) -> usize { debug_assert!( - self.0.is_main_thread(), + MainThreadMarker::new().is_some(), "this should only be called from the main thread" ); diff --git a/src/platform_impl/web/async/wrapper.rs b/src/platform_impl/web/async/wrapper.rs index 22088ef1fcb..0a5ff52fddf 100644 --- a/src/platform_impl/web/async/wrapper.rs +++ b/src/platform_impl/web/async/wrapper.rs @@ -2,8 +2,8 @@ use std::cell::{Ref, RefCell}; use std::future::Future; use std::marker::PhantomData; use std::sync::Arc; -use wasm_bindgen::prelude::wasm_bindgen; -use wasm_bindgen::{JsCast, JsValue}; + +use super::super::main_thread::MainThreadMarker; // Unsafe wrapper type that allows us to use `T` when it's not `Send` from other threads. // `value` **must** only be accessed on the main thread. @@ -34,36 +34,15 @@ unsafe impl Send for Value {} unsafe impl Sync for Value {} impl Wrapper { - thread_local! { - static MAIN_THREAD: bool = { - #[wasm_bindgen] - extern "C" { - #[derive(Clone)] - type Global; - - #[wasm_bindgen(method, getter, js_name = Window)] - fn window(this: &Global) -> JsValue; - } - - let global: Global = js_sys::global().unchecked_into(); - !global.window().is_undefined() - }; - } - #[track_caller] pub fn new>( + _: MainThreadMarker, value: V, handler: fn(&RefCell>, E), receiver: impl 'static + FnOnce(Arc>>) -> R, sender_data: S, sender_handler: fn(&S, E), ) -> Option { - Self::MAIN_THREAD.with(|safe| { - if !safe { - panic!("only callable from inside the `Window`") - } - }); - let value = Arc::new(RefCell::new(Some(value))); wasm_bindgen_futures::spawn_local({ @@ -86,29 +65,16 @@ impl Wrapper { } pub fn send(&self, event: E) { - Self::MAIN_THREAD.with(|is_main_thread| { - if *is_main_thread { - (self.handler)(&self.value.value, event) - } else { - (self.sender_handler)(&self.sender_data, event) - } - }) - } - - pub fn is_main_thread(&self) -> bool { - Self::MAIN_THREAD.with(|is_main_thread| *is_main_thread) + if MainThreadMarker::new().is_some() { + (self.handler)(&self.value.value, event) + } else { + (self.sender_handler)(&self.sender_data, event) + } } pub fn value(&self) -> Option> { - Self::MAIN_THREAD.with(|is_main_thread| { - if *is_main_thread { - Some(Ref::map(self.value.value.borrow(), |value| { - value.as_ref().unwrap() - })) - } else { - None - } - }) + MainThreadMarker::new() + .map(|_| Ref::map(self.value.value.borrow(), |value| value.as_ref().unwrap())) } pub fn with_sender_data(&self, f: impl FnOnce(&S) -> T) -> T { diff --git a/src/platform_impl/web/cursor.rs b/src/platform_impl/web/cursor.rs index cd8922ecc82..13f3b4fe061 100644 --- a/src/platform_impl/web/cursor.rs +++ b/src/platform_impl/web/cursor.rs @@ -9,12 +9,8 @@ use std::{ task::{Poll, Waker}, }; -use crate::{ - cursor::{BadImage, CursorImage}, - platform_impl::platform::r#async, -}; +use crate::cursor::{BadImage, CursorImage}; use cursor_icon::CursorIcon; -use once_cell::sync::Lazy; use wasm_bindgen::{closure::Closure, JsCast}; use wasm_bindgen_futures::JsFuture; use web_sys::{ @@ -22,9 +18,9 @@ use web_sys::{ ImageBitmapRenderingContext, ImageData, PremultiplyAlpha, Url, Window, }; -use self::thread_safe::ThreadSafe; - -use super::{backend::Style, r#async::AsyncSender, EventLoopWindowTarget}; +use super::backend::Style; +use super::main_thread::{MainThreadMarker, MainThreadSafe}; +use super::EventLoopWindowTarget; #[derive(Debug)] pub(crate) enum CustomCursorBuilder { @@ -51,7 +47,7 @@ impl CustomCursorBuilder { } #[derive(Clone, Debug)] -pub struct CustomCursor(Arc); +pub struct CustomCursor(Arc>>); impl Hash for CustomCursor { fn hash(&self, state: &mut H) { @@ -68,14 +64,22 @@ impl PartialEq for CustomCursor { impl Eq for CustomCursor {} impl CustomCursor { + fn new(main_thread: MainThreadMarker) -> Self { + Self(Arc::new(MainThreadSafe::new( + main_thread, + RefCell::new(ImageState::Loading(None)), + ))) + } + pub(crate) fn build( builder: CustomCursorBuilder, window_target: &EventLoopWindowTarget, ) -> Self { - Lazy::force(&DROP_HANDLER); + let main_thread = window_target.runner.main_thread(); - Self(match builder { + match builder { CustomCursorBuilder::Image(image) => ImageState::from_rgba( + main_thread, window_target.runner.window(), window_target.runner.document().clone(), &image, @@ -84,47 +88,7 @@ impl CustomCursor { url, hotspot_x, hotspot_y, - } => ImageState::from_url(url, hotspot_x, hotspot_y), - }) - } -} - -#[derive(Debug)] -struct Inner(Option>>); - -static DROP_HANDLER: Lazy>>> = Lazy::new(|| { - let (sender, receiver) = r#async::channel(); - wasm_bindgen_futures::spawn_local(async move { while receiver.next().await.is_ok() {} }); - - sender -}); - -impl Inner { - fn new() -> Arc { - Arc::new(Inner(Some(ThreadSafe::new(RefCell::new( - ImageState::Loading(None), - ))))) - } - - fn get(&self) -> &RefCell { - self.0 - .as_ref() - .expect("value has accidently already been dropped") - .get() - } -} - -impl Drop for Inner { - fn drop(&mut self) { - let value = self - .0 - .take() - .expect("value has accidently already been dropped"); - - if !value.in_origin_thread() { - DROP_HANDLER - .send(value) - .expect("sender dropped in main thread") + } => ImageState::from_url(main_thread, url, hotspot_x, hotspot_y), } } } @@ -133,8 +97,9 @@ impl Drop for Inner { pub struct CursorState(Rc>); impl CursorState { - pub fn new(style: Style) -> Self { + pub fn new(main_thread: MainThreadMarker, style: Style) -> Self { Self(Rc::new(RefCell::new(State { + main_thread, style, visible: true, cursor: SelectedCursor::default(), @@ -145,7 +110,9 @@ impl CursorState { let mut this = self.0.borrow_mut(); if let SelectedCursor::ImageLoading { state, .. } = &this.cursor { - if let ImageState::Loading(state) = state.get().borrow_mut().deref_mut() { + if let ImageState::Loading(state) = + state.0.get(this.main_thread).borrow_mut().deref_mut() + { state.take(); } } @@ -157,10 +124,10 @@ impl CursorState { pub(crate) fn set_custom_cursor(&self, cursor: CustomCursor) { let mut this = self.0.borrow_mut(); - match cursor.0.get().borrow_mut().deref_mut() { + match cursor.0.get(this.main_thread).borrow_mut().deref_mut() { ImageState::Loading(state) => { this.cursor = SelectedCursor::ImageLoading { - state: cursor.0.clone(), + state: cursor.clone(), previous: mem::take(&mut this.cursor).into(), }; *state = Some(Rc::downgrade(&self.0)); @@ -188,6 +155,7 @@ impl CursorState { #[derive(Debug)] struct State { + main_thread: MainThreadMarker, style: Style, visible: bool, cursor: SelectedCursor, @@ -211,7 +179,7 @@ impl State { enum SelectedCursor { Named(CursorIcon), ImageLoading { - state: Arc, + state: CustomCursor, previous: Previous, }, ImageReady(Rc), @@ -265,7 +233,12 @@ enum ImageState { } impl ImageState { - fn from_rgba(window: &Window, document: Document, image: &CursorImage) -> Arc { + fn from_rgba( + main_thread: MainThreadMarker, + window: &Window, + document: Document, + image: &CursorImage, + ) -> CustomCursor { // 1. Create an `ImageData` from the RGBA data. // 2. Create an `ImageBitmap` from the `ImageData`. // 3. Draw `ImageBitmap` on an `HTMLCanvasElement`. @@ -317,10 +290,11 @@ impl ImageState { .expect("unexpected exception in `createImageBitmap()`"), ); - let this = Inner::new(); + #[allow(clippy::arc_with_non_send_sync)] + let this = CustomCursor::new(main_thread); wasm_bindgen_futures::spawn_local({ - let weak = Arc::downgrade(&this); + let weak = Arc::downgrade(&this.0); let CursorImage { width, height, @@ -395,7 +369,7 @@ impl ImageState { let Some(this) = weak.upgrade() else { return; }; - let mut this = this.get().borrow_mut(); + let mut this = this.get(main_thread).borrow_mut(); let Some(blob) = blob else { log::error!("creating custom cursor failed"); @@ -423,17 +397,24 @@ impl ImageState { .expect("unexpected exception in `URL.createObjectURL()`") }; - Self::decode(weak, url, true, hotspot_x, hotspot_y).await; + Self::decode(main_thread, weak, url, true, hotspot_x, hotspot_y).await; } }); this } - fn from_url(url: String, hotspot_x: u16, hotspot_y: u16) -> Arc { - let this = Inner::new(); + fn from_url( + main_thread: MainThreadMarker, + url: String, + hotspot_x: u16, + hotspot_y: u16, + ) -> CustomCursor { + #[allow(clippy::arc_with_non_send_sync)] + let this = CustomCursor::new(main_thread); wasm_bindgen_futures::spawn_local(Self::decode( - Arc::downgrade(&this), + main_thread, + Arc::downgrade(&this.0), url, false, hotspot_x, @@ -444,7 +425,8 @@ impl ImageState { } async fn decode( - weak: sync::Weak, + main_thread: MainThreadMarker, + weak: sync::Weak>>, url: String, object: bool, hotspot_x: u16, @@ -463,7 +445,7 @@ impl ImageState { let Some(this) = weak.upgrade() else { return; }; - let mut this = this.get().borrow_mut(); + let mut this = this.get(main_thread).borrow_mut(); let ImageState::Loading(state) = this.deref_mut() else { unreachable!("found invalid state"); @@ -534,46 +516,3 @@ impl Image { }) } } - -mod thread_safe { - use std::mem; - use std::thread::{self, ThreadId}; - - #[derive(Debug)] - pub struct ThreadSafe { - origin_thread: ThreadId, - value: T, - } - - impl ThreadSafe { - pub fn new(value: T) -> Self { - Self { - origin_thread: thread::current().id(), - value, - } - } - - pub fn get(&self) -> &T { - if self.origin_thread == thread::current().id() { - &self.value - } else { - panic!("value not accessible outside its origin thread") - } - } - - pub fn in_origin_thread(&self) -> bool { - self.origin_thread == thread::current().id() - } - } - - impl Drop for ThreadSafe { - fn drop(&mut self) { - if mem::needs_drop::() && self.origin_thread != thread::current().id() { - panic!("value can't be dropped outside its origin thread") - } - } - } - - unsafe impl Send for ThreadSafe {} - unsafe impl Sync for ThreadSafe {} -} diff --git a/src/platform_impl/web/event_loop/runner.rs b/src/platform_impl/web/event_loop/runner.rs index b029ab516f8..663ebce4dd1 100644 --- a/src/platform_impl/web/event_loop/runner.rs +++ b/src/platform_impl/web/event_loop/runner.rs @@ -8,6 +8,7 @@ use crate::event::{ use crate::event_loop::{ControlFlow, DeviceEvents}; use crate::platform::web::PollStrategy; use crate::platform_impl::platform::backend::EventListenerHandle; +use crate::platform_impl::platform::main_thread::MainThreadMarker; use crate::platform_impl::platform::r#async::{DispatchRunner, Waker, WakerSpawner}; use crate::platform_impl::platform::window::Inner; use crate::window::WindowId; @@ -37,6 +38,7 @@ impl Clone for Shared { type OnEventHandle = RefCell>>; pub struct Execution { + main_thread: MainThreadMarker, proxy_spawner: WakerSpawner>, control_flow: Cell, poll_strategy: Cell, @@ -143,13 +145,14 @@ impl Runner { impl Shared { pub fn new() -> Self { + let main_thread = MainThreadMarker::new().expect("only callable from inside the `Window`"); #[allow(clippy::disallowed_methods)] let window = web_sys::window().expect("only callable from inside the `Window`"); #[allow(clippy::disallowed_methods)] let document = window.document().expect("Failed to obtain document"); Shared(Rc::::new_cyclic(|weak| { - let proxy_spawner = WakerSpawner::new(weak.clone(), |runner, count| { + let proxy_spawner = WakerSpawner::new(main_thread, weak.clone(), |runner, count| { if let Some(runner) = runner.upgrade() { Shared(runner).send_events(iter::repeat(Event::UserEvent(())).take(count)) } @@ -157,6 +160,7 @@ impl Shared { .expect("`EventLoop` has to be created in the main thread"); Execution { + main_thread, proxy_spawner, control_flow: Cell::new(ControlFlow::default()), poll_strategy: Cell::new(PollStrategy::default()), @@ -184,6 +188,10 @@ impl Shared { })) } + pub fn main_thread(&self) -> MainThreadMarker { + self.0.main_thread + } + pub fn window(&self) -> &web_sys::Window { &self.0.window } diff --git a/src/platform_impl/web/main_thread.rs b/src/platform_impl/web/main_thread.rs new file mode 100644 index 00000000000..8c8598f4766 --- /dev/null +++ b/src/platform_impl/web/main_thread.rs @@ -0,0 +1,96 @@ +use std::fmt::{self, Debug, Formatter}; +use std::marker::PhantomData; +use std::mem; +use std::sync::OnceLock; + +use wasm_bindgen::prelude::wasm_bindgen; +use wasm_bindgen::{JsCast, JsValue}; + +use super::r#async::{self, AsyncSender}; + +thread_local! { + static MAIN_THREAD: bool = { + #[wasm_bindgen] + extern "C" { + #[derive(Clone)] + type Global; + + #[wasm_bindgen(method, getter, js_name = Window)] + fn window(this: &Global) -> JsValue; + } + + let global: Global = js_sys::global().unchecked_into(); + !global.window().is_undefined() + }; +} + +#[derive(Clone, Copy, Debug)] +pub struct MainThreadMarker(PhantomData<*const ()>); + +impl MainThreadMarker { + pub fn new() -> Option { + MAIN_THREAD.with(|is| is.then_some(Self(PhantomData))) + } +} + +pub struct MainThreadSafe(Option); + +impl MainThreadSafe { + pub fn new(_: MainThreadMarker, value: T) -> Self { + DROP_HANDLER.get_or_init(|| { + let (sender, receiver) = r#async::channel(); + wasm_bindgen_futures::spawn_local( + async move { while receiver.next().await.is_ok() {} }, + ); + + sender + }); + + Self(Some(value)) + } + + pub fn into_inner(mut self, _: MainThreadMarker) -> T { + self.0.take().expect("already taken or dropped") + } + + pub fn get(&self, _: MainThreadMarker) -> &T { + self.0.as_ref().expect("already taken or dropped") + } +} + +impl Debug for MainThreadSafe { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + if MainThreadMarker::new().is_some() { + f.debug_tuple("MainThreadSafe").field(&self.0).finish() + } else { + f.debug_struct("MainThreadSafe").finish_non_exhaustive() + } + } +} + +impl Drop for MainThreadSafe { + fn drop(&mut self) { + if let Some(value) = self.0.take() { + if mem::needs_drop::() && MainThreadMarker::new().is_none() { + DROP_HANDLER + .get() + .expect("drop handler not initialized when setting canvas") + .send(DropBox(Box::new(value))) + .expect("sender dropped in main thread") + } + } + } +} + +unsafe impl Send for MainThreadSafe {} +unsafe impl Sync for MainThreadSafe {} + +static DROP_HANDLER: OnceLock> = OnceLock::new(); + +struct DropBox(Box); + +unsafe impl Send for DropBox {} +unsafe impl Sync for DropBox {} + +trait Any {} +impl Any for T {} diff --git a/src/platform_impl/web/mod.rs b/src/platform_impl/web/mod.rs index 56ca0eaf478..f3c063e2183 100644 --- a/src/platform_impl/web/mod.rs +++ b/src/platform_impl/web/mod.rs @@ -23,6 +23,7 @@ mod device; mod error; mod event_loop; mod keyboard; +mod main_thread; mod monitor; mod window; diff --git a/src/platform_impl/web/web_sys/canvas.rs b/src/platform_impl/web/web_sys/canvas.rs index 8d60f49a1ee..e1bc26eade8 100644 --- a/src/platform_impl/web/web_sys/canvas.rs +++ b/src/platform_impl/web/web_sys/canvas.rs @@ -14,6 +14,7 @@ use crate::dpi::{LogicalPosition, PhysicalPosition, PhysicalSize}; use crate::error::OsError as RootOE; use crate::event::{Force, InnerSizeWriter, MouseButton, MouseScrollDelta}; use crate::keyboard::{Key, KeyLocation, ModifiersState, PhysicalKey}; +use crate::platform_impl::platform::main_thread::MainThreadMarker; use crate::platform_impl::{OsError, PlatformSpecificWindowBuilderAttributes}; use crate::window::{WindowAttributes, WindowId as RootWindowId}; @@ -65,13 +66,18 @@ pub struct Style { impl Canvas { pub fn create( + main_thread: MainThreadMarker, id: WindowId, window: web_sys::Window, document: Document, attr: &WindowAttributes, - platform_attr: PlatformSpecificWindowBuilderAttributes, + mut platform_attr: PlatformSpecificWindowBuilderAttributes, ) -> Result { - let canvas = match platform_attr.canvas.0 { + let canvas = match platform_attr.canvas.take().map(|canvas| { + Arc::try_unwrap(canvas) + .map(|canvas| canvas.into_inner(main_thread)) + .unwrap_or_else(|canvas| canvas.get(main_thread).clone()) + }) { Some(canvas) => canvas, None => document .create_element("canvas") @@ -129,7 +135,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); } diff --git a/src/platform_impl/web/window.rs b/src/platform_impl/web/window.rs index f3845dc810e..fb2520bc6e1 100644 --- a/src/platform_impl/web/window.rs +++ b/src/platform_impl/web/window.rs @@ -5,9 +5,9 @@ use crate::window::{ CursorGrabMode, CursorIcon, ImePurpose, ResizeDirection, Theme, UserAttentionType, WindowAttributes, WindowButtons, WindowId as RootWI, WindowLevel, }; -use crate::SendSyncWrapper; use super::cursor::CursorState; +use super::main_thread::{MainThreadMarker, MainThreadSafe}; use super::r#async::Dispatcher; use super::PlatformCustomCursor; use super::{backend, monitor::MonitorHandle, EventLoopWindowTarget, Fullscreen}; @@ -16,6 +16,7 @@ use web_sys::HtmlCanvasElement; use std::cell::RefCell; use std::collections::VecDeque; use std::rc::Rc; +use std::sync::Arc; pub struct Window { inner: Dispatcher, @@ -41,10 +42,16 @@ impl Window { let window = target.runner.window(); let document = target.runner.document(); - let canvas = - backend::Canvas::create(id, window.clone(), document.clone(), &attr, platform_attr)?; + let canvas = backend::Canvas::create( + target.runner.main_thread(), + id, + window.clone(), + document.clone(), + &attr, + platform_attr, + )?; let canvas = Rc::new(RefCell::new(canvas)); - let cursor = CursorState::new(canvas.borrow().style().clone()); + let cursor = CursorState::new(target.runner.main_thread(), canvas.borrow().style().clone()); target.register(&canvas, id, prevent_default); @@ -65,7 +72,7 @@ impl Window { inner.set_window_icon(attr.window_icon); let canvas = Rc::downgrade(&inner.canvas); - let (dispatcher, runner) = Dispatcher::new(inner).unwrap(); + let (dispatcher, runner) = Dispatcher::new(target.runner.main_thread(), inner).unwrap(); target.runner.add_canvas(RootWI(id), canvas, runner); Ok(Window { inner: dispatcher }) @@ -100,6 +107,16 @@ impl Window { }) .ok_or(rwh_06::HandleError::Unavailable) } + + #[cfg(feature = "rwh_06")] + #[inline] + pub(crate) fn raw_display_handle_rwh_06( + &self, + ) -> Result { + Ok(rwh_06::RawDisplayHandle::Web( + rwh_06::WebDisplayHandle::new(), + )) + } } impl Inner { @@ -393,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 { - Ok(rwh_06::RawDisplayHandle::Web( - rwh_06::WebDisplayHandle::new(), - )) - } - #[inline] pub fn set_theme(&self, _theme: Option) {} @@ -463,16 +470,30 @@ impl From for WindowId { #[derive(Clone)] pub struct PlatformSpecificWindowBuilderAttributes { - pub(crate) canvas: SendSyncWrapper>, + pub(crate) canvas: Option>>, pub(crate) prevent_default: bool, pub(crate) focusable: bool, pub(crate) append: bool, } +impl PlatformSpecificWindowBuilderAttributes { + pub(crate) fn set_canvas(&mut self, canvas: Option) { + let Some(canvas) = canvas else { + self.canvas = None; + return; + }; + + let main_thread = MainThreadMarker::new() + .expect("received a `HtmlCanvasElement` outside the window context"); + + self.canvas = Some(Arc::new(MainThreadSafe::new(main_thread, canvas))); + } +} + impl Default for PlatformSpecificWindowBuilderAttributes { fn default() -> Self { Self { - canvas: SendSyncWrapper(None), + canvas: None, prevent_default: true, focusable: true, append: false, diff --git a/src/platform_impl/windows/window.rs b/src/platform_impl/windows/window.rs index 65b879c62d3..c5b2c364126 100644 --- a/src/platform_impl/windows/window.rs +++ b/src/platform_impl/windows/window.rs @@ -1226,8 +1226,8 @@ impl<'a, T: 'static> InitData<'a, T> { win.set_enabled_buttons(attributes.enabled_buttons); - if attributes.fullscreen.0.is_some() { - win.set_fullscreen(attributes.fullscreen.0.map(Into::into)); + if attributes.fullscreen.is_some() { + win.set_fullscreen(attributes.fullscreen.map(Into::into)); unsafe { force_window_active(win.window) }; } else { let size = attributes @@ -1313,7 +1313,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() { diff --git a/src/window.rs b/src/window.rs index 87864352727..41dfea1a19d 100644 --- a/src/window.rs +++ b/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, CustomCursor, CustomCursorBuilder, MAX_CURSOR_SIZE}; @@ -154,8 +154,8 @@ pub struct WindowAttributes { pub window_level: WindowLevel, pub active: bool, #[cfg(feature = "rwh_06")] - pub(crate) parent_window: SendSyncWrapper>, - pub(crate) fullscreen: SendSyncWrapper>, + pub(crate) parent_window: Option, + pub fullscreen: Option, } impl Default for WindowAttributes { @@ -170,7 +170,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, @@ -181,22 +181,32 @@ impl Default for WindowAttributes { resize_increments: None, content_protected: false, #[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() - } - - /// Get `Fullscreen` option stored on the attributes. - pub fn fullscreen(&self) -> Option<&Fullscreen> { - self.fullscreen.0.as_ref() + self.parent_window.as_ref().map(|handle| &handle.0) } } @@ -317,7 +327,7 @@ impl WindowBuilder { /// See [`Window::set_fullscreen`] for details. #[inline] pub fn with_fullscreen(mut self, fullscreen: Option) -> Self { - self.window.fullscreen = SendSyncWrapper(fullscreen); + self.window.fullscreen = fullscreen; self } @@ -493,7 +503,7 @@ impl WindowBuilder { mut self, parent_window: Option, ) -> Self { - self.window.parent_window = SendSyncWrapper(parent_window); + self.window.parent_window = parent_window.map(SendSyncRawWindowHandle); self } @@ -1541,21 +1551,29 @@ impl rwh_06::HasWindowHandle for Window { #[cfg(feature = "rwh_06")] impl rwh_06::HasDisplayHandle for Window { fn display_handle(&self) -> Result, rwh_06::HandleError> { - let raw = self - .window - .maybe_wait_on_main(|w| w.raw_display_handle_rwh_06().map(SendSyncWrapper))? - .0; + let raw = self.window.raw_display_handle_rwh_06()?; - // 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); + +unsafe impl Send for UnsafeSendWrapper {} + #[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 } } @@ -1568,7 +1586,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 } } @@ -1577,7 +1595,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 } }