diff --git a/CHANGELOG.md b/CHANGELOG.md index 3979c0cc..2c64e3e6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,6 +42,7 @@ - On Wayland, add `set_wayland_theme()` to control client decoration color theme - Added serde serialization to `os::unix::XWindowType`. - **Breaking:** Remove the `icon_loading` feature and the associated `image` dependency. +- On X11, make event loop thread safe by replacing XNextEvent with select(2) and XCheckIfEvent - On Windows, fix malformed function pointer typecast that could invoke undefined behavior. - Refactored Windows state/flag-setting code. - On Windows, hiding the cursor no longer hides the cursor for all Winit windows - just the one `hide_cursor` was called on. diff --git a/src/platform_impl/linux/x11/mod.rs b/src/platform_impl/linux/x11/mod.rs index 1e2d5930..a98556c1 100644 --- a/src/platform_impl/linux/x11/mod.rs +++ b/src/platform_impl/linux/x11/mod.rs @@ -19,6 +19,7 @@ use std::collections::HashMap; use std::ffi::CStr; use std::ops::Deref; use std::os::raw::*; +use libc::{select, fd_set, FD_SET, FD_ZERO, FD_ISSET, EINTR, EINVAL, ENOMEM, EBADF, __errno_location}; use std::sync::{Arc, mpsc, Weak}; use std::sync::atomic::{self, AtomicBool}; @@ -185,6 +186,70 @@ impl EventLoop { } } + unsafe fn poll_one_event(&mut self, event_ptr : *mut ffi::XEvent) -> bool { + // This function is used to poll and remove a single event + // from the Xlib event queue in a non-blocking, atomic way. + // XCheckIfEvent is non-blocking and removes events from queue. + // XNextEvent can't be used because it blocks while holding the + // global Xlib mutex. + // XPeekEvent does not remove events from the queue. + unsafe extern "C" fn predicate( + _display: *mut ffi::Display, + _event: *mut ffi::XEvent, + _arg : *mut c_char) -> c_int { + // This predicate always returns "true" (1) to accept all events + 1 + } + + let result = (self.xconn.xlib.XCheckIfEvent)( + self.xconn.display, + event_ptr, + Some(predicate), + std::ptr::null_mut()); + + result != 0 + } + + unsafe fn wait_for_input(&mut self) { + // XNextEvent can not be used in multi-threaded applications + // because it is blocking for input while holding the global + // Xlib mutex. + // To work around this issue, first flush the X11 display, then + // use select(2) to wait for input to arrive + loop { + // First use XFlush to flush any buffered x11 requests + (self.xconn.xlib.XFlush)(self.xconn.display); + + // Then use select(2) to wait for input data + let mut fds : fd_set = mem::uninitialized(); + FD_ZERO(&mut fds); + FD_SET(self.xconn.x11_fd, &mut fds); + let err = select( + self.xconn.x11_fd + 1, + &mut fds, // read fds + std::ptr::null_mut(), // write fds + std::ptr::null_mut(), // except fds (could be used to detect errors) + std::ptr::null_mut()); // timeout + + if err < 0 { + let errno_ptr = __errno_location(); + let errno = *errno_ptr; + + if errno == EINTR { + // try again if errno is EINTR + continue; + } + + assert!(errno == EBADF || errno == EINVAL || errno == ENOMEM); + panic!("select(2) returned fatal error condition"); + } + + if FD_ISSET(self.xconn.x11_fd, &mut fds) { + break; + } + } + } + pub fn poll_events(&mut self, mut callback: F) where F: FnMut(Event) { @@ -192,13 +257,9 @@ impl EventLoop { loop { // Get next event unsafe { - // Ensure XNextEvent won't block - let count = (self.xconn.xlib.XPending)(self.xconn.display); - if count == 0 { + if !self.poll_one_event(&mut xev) { break; } - - (self.xconn.xlib.XNextEvent)(self.xconn.display, &mut xev); } self.process_event(&mut xev, &mut callback); } @@ -210,7 +271,12 @@ impl EventLoop { let mut xev = unsafe { mem::uninitialized() }; loop { - unsafe { (self.xconn.xlib.XNextEvent)(self.xconn.display, &mut xev) }; // Blocks as necessary + unsafe { + while !self.poll_one_event(&mut xev) { + // block until input is available + self.wait_for_input(); + } + }; let mut control_flow = ControlFlow::Continue; diff --git a/src/platform_impl/linux/x11/xdisplay.rs b/src/platform_impl/linux/x11/xdisplay.rs index c0f1e7d2..cceaa8c7 100644 --- a/src/platform_impl/linux/x11/xdisplay.rs +++ b/src/platform_impl/linux/x11/xdisplay.rs @@ -1,6 +1,7 @@ use std::ptr; use std::fmt; use std::error::Error; +use std::os::raw::c_int; use libc; use parking_lot::Mutex; @@ -18,6 +19,7 @@ pub struct XConnection { pub xinput2: ffi::XInput2, pub xlib_xcb: ffi::Xlib_xcb, pub display: *mut ffi::Display, + pub x11_fd: c_int, pub latest_error: Mutex>, } @@ -48,6 +50,11 @@ impl XConnection { display }; + // Get X11 socket file descriptor + let fd = unsafe { + (xlib.XConnectionNumber)(display) + }; + Ok(XConnection { xlib, xrandr, @@ -56,6 +63,7 @@ impl XConnection { xinput2, xlib_xcb, display, + x11_fd: fd, latest_error: Mutex::new(None), }) }