Skip to content

Commit

Permalink
api: make OwnedDisplayHandle wrap an opaque type
Browse files Browse the repository at this point in the history
This will help in case we want to linger the event loop during drop
to prevent use after free in the consumers code.
  • Loading branch information
kchibisov authored Nov 13, 2024
1 parent f781e13 commit 59b1eb5
Show file tree
Hide file tree
Showing 19 changed files with 149 additions and 177 deletions.
48 changes: 29 additions & 19 deletions src/event_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -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())
}
}

Expand Down Expand Up @@ -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()
}
}
Expand All @@ -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);
Expand Down
16 changes: 7 additions & 9 deletions src/platform_impl/android/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand All @@ -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()) })
}
}

Expand Down
19 changes: 9 additions & 10 deletions src/platform_impl/apple/appkit/event_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)) }
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/platform_impl/apple/appkit/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
18 changes: 9 additions & 9 deletions src/platform_impl/apple/uikit/event_loop.rs
Original file line number Diff line number Diff line change
@@ -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::{
Expand All @@ -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};
Expand All @@ -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;
Expand Down Expand Up @@ -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 {
Expand All @@ -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)) }
}
}

Expand Down
3 changes: 1 addition & 2 deletions src/platform_impl/apple/uikit/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down
52 changes: 0 additions & 52 deletions src/platform_impl/linux/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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`)
Expand Down
Loading

0 comments on commit 59b1eb5

Please sign in to comment.