x11: thread safe replacement for XNextEvent (#782)

XNextEvent will block for input while holding the global Xlib mutex.

This will cause a deadlock in even the most trivial multi-threaded
application because OpenGL functions will need to hold the Xlib mutex
too.

Add EventsLoop::poll_one_event and EventsLoop::wait_for_input to provide
thread-safe functions to poll and wait events from the X11 event queue
using unix select(2) and XCheckIfEvent.

This is a somewhat ugly workaround to an ugly problem.

Fixes #779

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.

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<F>(&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;
 

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<Option<XError>>,
 }
 
@@ -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),
         })
     }