From 1ec976f95eadd3369cbca466e4a8e8243f8fafdc Mon Sep 17 00:00:00 2001 From: Kirill Chibisov Date: Fri, 22 Jul 2022 20:21:28 +0300 Subject: [PATCH] Add method to hook xlib error handler This should help glutin to handle errors coming from GLX and offer multithreading support in a safe way. Fixes #2378. --- CHANGELOG.md | 1 + src/platform/unix.rs | 25 ++++++++++++++++++++++++- src/platform_impl/linux/mod.rs | 17 ++++++++++++++++- 3 files changed, 41 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0a024d7866..d13fccb555 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -73,6 +73,7 @@ And please only add new entries to the top of this list, right below the `# Unre - On Web, add `with_prevent_default` and `with_focusable` to `WindowBuilderExtWebSys` to control whether events should be propagated. - On Windows, fix focus events being sent to inactive windows. - **Breaking**, update `raw-window-handle` to `v0.5` and implement `HasRawDisplayHandle` for `Window` and `EventLoopWindowTarget`. +- On X11, add function `register_xlib_error_hook` into `winit::platform::unix` to subscribe for errors comming from Xlib. # 0.26.1 (2022-01-05) diff --git a/src/platform/unix.rs b/src/platform/unix.rs index ae0c994252..591f038f25 100644 --- a/src/platform/unix.rs +++ b/src/platform/unix.rs @@ -19,7 +19,7 @@ use crate::{ #[cfg(feature = "x11")] use crate::dpi::Size; #[cfg(feature = "x11")] -use crate::platform_impl::x11::{ffi::XVisualInfo, XConnection}; +use crate::platform_impl::{x11::ffi::XVisualInfo, x11::XConnection, XLIB_ERROR_HOOKS}; use crate::platform_impl::{ ApplicationName, Backend, EventLoopWindowTarget as LinuxEventLoopWindowTarget, Window as LinuxWindow, @@ -35,6 +35,29 @@ pub use crate::platform_impl::{x11::util::WindowType as XWindowType, XNotSupport #[cfg(feature = "wayland")] pub use crate::window::Theme; +/// The first argument in the provided hook will be the pointer to XDisplay +/// and the second one the pointer to XError. The return `bool` is an indicator +/// whether error was handled by the callback. +#[cfg(feature = "x11")] +pub type XlibErrorHook = + Box bool + Send + Sync>; + +/// Hook to winit's xlib error handling callback. +/// +/// This method is provided as a safe way to handle the errors comming from X11 when using xlib +/// in external crates, like glutin for GLX access. Trying to handle errors by speculating with +/// `XSetErrorHandler` is [`unsafe`]. +/// +/// [`unsafe`]: https://www.remlab.net/op/xlib.shtml +#[inline] +#[cfg(feature = "x11")] +pub fn register_xlib_error_hook(hook: XlibErrorHook) { + // Append new hook. + unsafe { + XLIB_ERROR_HOOKS.lock().push(hook); + } +} + /// Additional methods on [`EventLoopWindowTarget`] that are specific to Unix. pub trait EventLoopWindowTargetExtUnix { /// True if the [`EventLoopWindowTarget`] uses Wayland. diff --git a/src/platform_impl/linux/mod.rs b/src/platform_impl/linux/mod.rs index 989bfec5e3..d73eb725ff 100644 --- a/src/platform_impl/linux/mod.rs +++ b/src/platform_impl/linux/mod.rs @@ -26,6 +26,8 @@ use raw_window_handle::{RawDisplayHandle, RawWindowHandle}; pub use self::x11::XNotSupported; #[cfg(feature = "x11")] use self::x11::{ffi::XVisualInfo, util::WindowType as XWindowType, XConnection, XError}; +#[cfg(feature = "x11")] +use crate::platform::unix::XlibErrorHook; #[cfg(feature = "wayland")] use crate::window::Theme; use crate::{ @@ -583,6 +585,10 @@ impl Window { } } +/// Hooks for X11 errors. +#[cfg(feature = "x11")] +pub(crate) static mut XLIB_ERROR_HOOKS: Mutex> = Mutex::new(Vec::new()); + #[cfg(feature = "x11")] unsafe extern "C" fn x_error_callback( display: *mut x11::ffi::Display, @@ -590,6 +596,12 @@ unsafe extern "C" fn x_error_callback( ) -> c_int { let xconn_lock = X11_BACKEND.lock(); if let Ok(ref xconn) = *xconn_lock { + // Call all the hooks. + let mut error_handled = false; + for hook in XLIB_ERROR_HOOKS.lock().iter() { + error_handled |= hook(display as *mut _, event as *mut _); + } + // `assume_init` is safe here because the array consists of `MaybeUninit` values, // which do not require initialization. let mut buf: [MaybeUninit; 1024] = MaybeUninit::uninit().assume_init(); @@ -608,7 +620,10 @@ unsafe extern "C" fn x_error_callback( minor_code: (*event).minor_code, }; - error!("X11 error: {:#?}", error); + // Don't log error. + if !error_handled { + error!("X11 error: {:#?}", error); + } *xconn.latest_error.lock() = Some(error); }