api: make OwnedDisplayHandle wrap an opaque type

This will help in case we want to linger the event loop during drop to prevent use after free in the consumers code.
rust-windowing · Nov 12, 2024 · b5b7ffd · b5b7ffd
1 parent 3a60cba
commit b5b7ffd
Show file tree

Hide file tree

Showing 19 changed files with 149 additions and 177 deletions.
diff --git a/src/event_loop.rs b/src/event_loop.rs
@@ -17,6 +17,7 @@ use std::sync::Arc;
 #[cfg(not(web_platform))]
 use std::time::{Duration, Instant};
 
+use rwh_06::{DisplayHandle, HandleError, HasDisplayHandle};
 #[cfg(web_platform)]
 use web_time::{Duration, Instant};
 
@@ -274,9 +275,9 @@ impl EventLoop {
     }
 }
 
-impl rwh_06::HasDisplayHandle for EventLoop {
-    fn display_handle(&self) -> Result<rwh_06::DisplayHandle<'_>, rwh_06::HandleError> {
-        rwh_06::HasDisplayHandle::display_handle(self.event_loop.window_target().rwh_06_handle())
+impl HasDisplayHandle for EventLoop {
+    fn display_handle(&self) -> Result<DisplayHandle<'_>, HandleError> {
+        HasDisplayHandle::display_handle(self.event_loop.window_target().rwh_06_handle())
     }
 }
 
@@ -407,11 +408,11 @@ pub trait ActiveEventLoop: AsAny {
     fn owned_display_handle(&self) -> OwnedDisplayHandle;
 
     /// Get the raw-window-handle handle.
-    fn rwh_06_handle(&self) -> &dyn rwh_06::HasDisplayHandle;
+    fn rwh_06_handle(&self) -> &dyn HasDisplayHandle;
 }
 
-impl rwh_06::HasDisplayHandle for dyn ActiveEventLoop + '_ {
-    fn display_handle(&self) -> Result<rwh_06::DisplayHandle<'_>, rwh_06::HandleError> {
+impl HasDisplayHandle for dyn ActiveEventLoop + '_ {
+    fn display_handle(&self) -> Result<DisplayHandle<'_>, HandleError> {
         self.rwh_06_handle().display_handle()
     }
 }
@@ -428,31 +429,40 @@ impl rwh_06::HasDisplayHandle for dyn ActiveEventLoop + '_ {
 ///
 /// - A zero-sized type that is likely optimized out.
 /// - A reference-counted pointer to the underlying type.
-#[derive(Clone, PartialEq, Eq)]
+#[derive(Clone)]
 pub struct OwnedDisplayHandle {
-    #[allow(dead_code)]
-    pub(crate) platform: platform_impl::OwnedDisplayHandle,
+    pub(crate) handle: Arc<dyn HasDisplayHandle>,
+}
+
+impl OwnedDisplayHandle {
+    pub(crate) fn new(handle: Arc<dyn HasDisplayHandle>) -> Self {
+        Self { handle }
+    }
+}
+
+impl HasDisplayHandle for OwnedDisplayHandle {
+    fn display_handle(&self) -> Result<DisplayHandle<'_>, HandleError> {
+        self.handle.display_handle()
+    }
 }
 
 impl fmt::Debug for OwnedDisplayHandle {
-    #[inline]
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         f.debug_struct("OwnedDisplayHandle").finish_non_exhaustive()
     }
 }
 
-impl rwh_06::HasDisplayHandle for OwnedDisplayHandle {
-    #[inline]
-    fn display_handle(&self) -> Result<rwh_06::DisplayHandle<'_>, rwh_06::HandleError> {
-        let raw = self.platform.raw_display_handle_rwh_06()?;
-
-        // SAFETY: The underlying display handle should be safe.
-        let handle = unsafe { rwh_06::DisplayHandle::borrow_raw(raw) };
-
-        Ok(handle)
+impl PartialEq for OwnedDisplayHandle {
+    fn eq(&self, other: &Self) -> bool {
+        match (self.display_handle(), other.display_handle()) {
+            (Ok(lhs), Ok(rhs)) => lhs == rhs,
+            _ => false,
+        }
     }
 }
 
+impl Eq for OwnedDisplayHandle {}
+
 pub(crate) trait EventLoopProxyProvider: Send + Sync {
     /// See [`EventLoopProxy::wake_up`] for details.
     fn wake_up(&self);

diff --git a/src/platform_impl/android/mod.rs b/src/platform_impl/android/mod.rs
@@ -19,7 +19,7 @@ use crate::event::{self, DeviceId, FingerId, Force, StartCause, SurfaceSizeWrite
 use crate::event_loop::{
     ActiveEventLoop as RootActiveEventLoop, ControlFlow, DeviceEvents,
     EventLoopProxy as CoreEventLoopProxy, EventLoopProxyProvider,
-    OwnedDisplayHandle as RootOwnedDisplayHandle,
+    OwnedDisplayHandle as CoreOwnedDisplayHandle,
 };
 use crate::monitor::MonitorHandle as RootMonitorHandle;
 use crate::platform::pump_events::PumpStatus;
@@ -718,8 +718,8 @@ impl RootActiveEventLoop for ActiveEventLoop {
         self.exit.get()
     }
 
-    fn owned_display_handle(&self) -> RootOwnedDisplayHandle {
-        RootOwnedDisplayHandle { platform: OwnedDisplayHandle }
+    fn owned_display_handle(&self) -> CoreOwnedDisplayHandle {
+        CoreOwnedDisplayHandle::new(Arc::new(OwnedDisplayHandle))
     }
 
     fn rwh_06_handle(&self) -> &dyn rwh_06::HasDisplayHandle {
@@ -737,12 +737,10 @@ impl rwh_06::HasDisplayHandle for ActiveEventLoop {
 #[derive(Clone, PartialEq, Eq)]
 pub(crate) struct OwnedDisplayHandle;
 
-impl OwnedDisplayHandle {
-    #[inline]
-    pub fn raw_display_handle_rwh_06(
-        &self,
-    ) -> Result<rwh_06::RawDisplayHandle, rwh_06::HandleError> {
-        Ok(rwh_06::AndroidDisplayHandle::new().into())
+impl rwh_06::HasDisplayHandle for OwnedDisplayHandle {
+    fn display_handle(&self) -> Result<rwh_06::DisplayHandle<'_>, rwh_06::HandleError> {
+        let raw = rwh_06::AndroidDisplayHandle::new();
+        Ok(unsafe { rwh_06::DisplayHandle::borrow_raw(raw.into()) })
     }
 }
 

diff --git a/src/platform_impl/apple/appkit/event_loop.rs b/src/platform_impl/apple/appkit/event_loop.rs
@@ -5,6 +5,7 @@ use std::panic::{catch_unwind, resume_unwind, RefUnwindSafe, UnwindSafe};
 use std::ptr;
 use std::rc::{Rc, Weak};
 use std::sync::atomic::{AtomicBool, Ordering as AtomicOrdering};
+use std::sync::Arc;
 use std::time::{Duration, Instant};
 
 use core_foundation::base::{CFIndex, CFRelease};
@@ -19,6 +20,7 @@ use objc2_app_kit::{
     NSApplicationWillTerminateNotification, NSWindow,
 };
 use objc2_foundation::{MainThreadMarker, NSNotificationCenter, NSObject, NSObjectProtocol};
+use rwh_06::HasDisplayHandle;
 
 use super::super::notification_center::create_observer;
 use super::app::WinitApplication;
@@ -32,7 +34,7 @@ use crate::error::{EventLoopError, RequestError};
 use crate::event_loop::{
     ActiveEventLoop as RootActiveEventLoop, ControlFlow, DeviceEvents,
     EventLoopProxy as CoreEventLoopProxy, EventLoopProxyProvider,
-    OwnedDisplayHandle as RootOwnedDisplayHandle,
+    OwnedDisplayHandle as CoreOwnedDisplayHandle,
 };
 use crate::monitor::MonitorHandle as RootMonitorHandle;
 use crate::platform::macos::ActivationPolicy;
@@ -150,8 +152,8 @@ impl RootActiveEventLoop for ActiveEventLoop {
         self.app_state.exiting()
     }
 
-    fn owned_display_handle(&self) -> RootOwnedDisplayHandle {
-        RootOwnedDisplayHandle { platform: OwnedDisplayHandle }
+    fn owned_display_handle(&self) -> CoreOwnedDisplayHandle {
+        CoreOwnedDisplayHandle::new(Arc::new(OwnedDisplayHandle))
     }
 
     fn rwh_06_handle(&self) -> &dyn rwh_06::HasDisplayHandle {
@@ -388,15 +390,12 @@ impl EventLoop {
     }
 }
 
-#[derive(Clone, PartialEq, Eq)]
 pub(crate) struct OwnedDisplayHandle;
 
-impl OwnedDisplayHandle {
-    #[inline]
-    pub fn raw_display_handle_rwh_06(
-        &self,
-    ) -> Result<rwh_06::RawDisplayHandle, rwh_06::HandleError> {
-        Ok(rwh_06::AppKitDisplayHandle::new().into())
+impl HasDisplayHandle for OwnedDisplayHandle {
+    fn display_handle(&self) -> Result<rwh_06::DisplayHandle<'_>, rwh_06::HandleError> {
+        let raw = rwh_06::RawDisplayHandle::AppKit(rwh_06::AppKitDisplayHandle::new());
+        unsafe { Ok(rwh_06::DisplayHandle::borrow_raw(raw)) }
     }
 }
 

diff --git a/src/platform_impl/apple/appkit/mod.rs b/src/platform_impl/apple/appkit/mod.rs
@@ -17,7 +17,7 @@ mod window_delegate;
 pub(crate) use self::cursor::CustomCursor as PlatformCustomCursor;
 pub(crate) use self::event::{physicalkey_to_scancode, scancode_to_physicalkey, KeyEventExtra};
 pub(crate) use self::event_loop::{
-    ActiveEventLoop, EventLoop, OwnedDisplayHandle, PlatformSpecificEventLoopAttributes,
+    ActiveEventLoop, EventLoop, PlatformSpecificEventLoopAttributes,
 };
 pub(crate) use self::monitor::{MonitorHandle, VideoModeHandle};
 pub(crate) use self::window::Window;

diff --git a/src/platform_impl/apple/uikit/event_loop.rs b/src/platform_impl/apple/uikit/event_loop.rs
@@ -1,6 +1,7 @@
 use std::ffi::{c_char, c_int, c_void};
 use std::ptr::{self, NonNull};
 use std::sync::atomic::{AtomicBool, Ordering as AtomicOrdering};
+use std::sync::Arc;
 
 use core_foundation::base::{CFIndex, CFRelease};
 use core_foundation::runloop::{
@@ -19,6 +20,7 @@ use objc2_ui_kit::{
     UIApplicationWillEnterForegroundNotification, UIApplicationWillResignActiveNotification,
     UIApplicationWillTerminateNotification, UIScreen,
 };
+use rwh_06::HasDisplayHandle;
 
 use super::super::notification_center::create_observer;
 use super::app_state::{send_occluded_event_for_all_windows, AppState, EventWrapper};
@@ -29,7 +31,7 @@ use crate::event::Event;
 use crate::event_loop::{
     ActiveEventLoop as RootActiveEventLoop, ControlFlow, DeviceEvents,
     EventLoopProxy as CoreEventLoopProxy, EventLoopProxyProvider,
-    OwnedDisplayHandle as RootOwnedDisplayHandle,
+    OwnedDisplayHandle as CoreOwnedDisplayHandle,
 };
 use crate::monitor::MonitorHandle as RootMonitorHandle;
 use crate::platform_impl::Window;
@@ -93,8 +95,8 @@ impl RootActiveEventLoop for ActiveEventLoop {
         false
     }
 
-    fn owned_display_handle(&self) -> RootOwnedDisplayHandle {
-        RootOwnedDisplayHandle { platform: OwnedDisplayHandle }
+    fn owned_display_handle(&self) -> CoreOwnedDisplayHandle {
+        CoreOwnedDisplayHandle::new(Arc::new(OwnedDisplayHandle))
     }
 
     fn rwh_06_handle(&self) -> &dyn rwh_06::HasDisplayHandle {
@@ -112,12 +114,10 @@ impl rwh_06::HasDisplayHandle for ActiveEventLoop {
 #[derive(Clone, PartialEq, Eq)]
 pub(crate) struct OwnedDisplayHandle;
 
-impl OwnedDisplayHandle {
-    #[inline]
-    pub fn raw_display_handle_rwh_06(
-        &self,
-    ) -> Result<rwh_06::RawDisplayHandle, rwh_06::HandleError> {
-        Ok(rwh_06::UiKitDisplayHandle::new().into())
+impl HasDisplayHandle for OwnedDisplayHandle {
+    fn display_handle(&self) -> Result<rwh_06::DisplayHandle<'_>, rwh_06::HandleError> {
+        let raw = rwh_06::RawDisplayHandle::UiKit(rwh_06::UiKitDisplayHandle::new());
+        unsafe { Ok(rwh_06::DisplayHandle::borrow_raw(raw)) }
     }
 }
 

diff --git a/src/platform_impl/apple/uikit/mod.rs b/src/platform_impl/apple/uikit/mod.rs
@@ -10,8 +10,7 @@ mod window;
 use std::fmt;
 
 pub(crate) use self::event_loop::{
-    ActiveEventLoop, EventLoop, EventLoopProxy, OwnedDisplayHandle,
-    PlatformSpecificEventLoopAttributes,
+    ActiveEventLoop, EventLoop, EventLoopProxy, PlatformSpecificEventLoopAttributes,
 };
 pub(crate) use self::monitor::{MonitorHandle, VideoModeHandle};
 pub(crate) use self::window::{PlatformSpecificWindowAttributes, Window};

diff --git a/src/platform_impl/linux/mod.rs b/src/platform_impl/linux/mod.rs
@@ -396,58 +396,6 @@ impl AsRawFd for EventLoop {
     }
 }
 
-#[derive(Clone)]
-#[allow(dead_code)]
-pub(crate) enum OwnedDisplayHandle {
-    #[cfg(x11_platform)]
-    X(Arc<XConnection>),
-    #[cfg(wayland_platform)]
-    Wayland(wayland_client::Connection),
-}
-
-impl OwnedDisplayHandle {
-    #[inline]
-    pub fn raw_display_handle_rwh_06(
-        &self,
-    ) -> Result<rwh_06::RawDisplayHandle, rwh_06::HandleError> {
-        use std::ptr::NonNull;
-
-        match self {
-            #[cfg(x11_platform)]
-            Self::X(xconn) => Ok(rwh_06::XlibDisplayHandle::new(
-                NonNull::new(xconn.display.cast()),
-                xconn.default_screen_index() as _,
-            )
-            .into()),
-
-            #[cfg(wayland_platform)]
-            Self::Wayland(conn) => {
-                use sctk::reexports::client::Proxy;
-
-                Ok(rwh_06::WaylandDisplayHandle::new(
-                    NonNull::new(conn.display().id().as_ptr().cast()).unwrap(),
-                )
-                .into())
-            },
-        }
-    }
-}
-
-impl PartialEq for OwnedDisplayHandle {
-    fn eq(&self, other: &Self) -> bool {
-        match (self, other) {
-            #[cfg(x11_platform)]
-            (Self::X(this), Self::X(other)) => Arc::as_ptr(this).eq(&Arc::as_ptr(other)),
-            #[cfg(wayland_platform)]
-            (Self::Wayland(this), Self::Wayland(other)) => this.eq(other),
-            #[cfg(all(x11_platform, wayland_platform))]
-            _ => false,
-        }
-    }
-}
-
-impl Eq for OwnedDisplayHandle {}
-
 /// Returns the minimum `Option<Duration>`, taking into account that `None`
 /// equates to an infinite timeout, not a zero timeout (so can't just use
 /// `Option::min`)