Skip to content

Commit

Permalink
Add method to hook xlib error handler
Browse files Browse the repository at this point in the history
This should help glutin to handle errors coming from GLX
and provide multithreading access in a safe way.

Fixes rust-windowing#2378.
  • Loading branch information
kchibisov committed Jul 21, 2022
1 parent f10ef5f commit 7b98a70
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 2 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
24 changes: 23 additions & 1 deletion src/platform/unix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -35,6 +35,28 @@ 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 = fn(display: *mut std::ffi::c_void, error: *mut std::ffi::c_void) -> bool;

/// 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.
Expand Down
17 changes: 16 additions & 1 deletion src/platform_impl/linux/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -583,13 +585,23 @@ impl Window {
}
}

/// Hooks for X11 errors.
#[cfg(feature = "x11")]
pub(crate) static mut XLIB_ERROR_HOOKS: Mutex<Vec<XlibErrorHook>> = Mutex::new(Vec::new());

#[cfg(feature = "x11")]
unsafe extern "C" fn x_error_callback(
display: *mut x11::ffi::Display,
event: *mut x11::ffi::XErrorEvent,
) -> 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<c_char>; 1024] = MaybeUninit::uninit().assume_init();
Expand All @@ -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);
}
Expand Down

0 comments on commit 7b98a70

Please sign in to comment.