Prevent EventLoop from getting initialized outside the main thread in cross-platform functions (#1186)

* Prevent EventLoop from getting initialized outside the main thread

This only applies to the cross-platform functions. We expose functions
to do this in a platform-specific manner, when available.

* Add CHANGELOG entry

* Formatting has changed since the latest stable update...

* Fix error spacing

* Unix: Prevent initializing EventLoop outside main thread

* Updates libc dependency to 0.2.64, as required by BSD platforms

* Update CHANGELOG.md for Linux implementation

* Finish sentence

* Consolidate documentation

CHANGELOG.md

@@ -35,6 +35,10 @@
 - On X11, fix use-after-free during window creation
 - On Windows, disable monitor change keyboard shortcut while in exclusive fullscreen.
 - On Windows, ensure that changing a borderless fullscreen window's monitor via keyboard shortcuts keeps the window fullscreen on the new monitor.
+- Prevent `EventLoop::new` and `EventLoop::with_user_event` from getting called outside the main thread.
+  - This is because some platforms cannot run the event loop outside the main thread. Preventing this
+    reduces the potential for cross-platform compatibility gotchyas.
+- On Windows and Linux X11/Wayland, add platform-specific functions for creating an `EventLoop` outside the main thread.
 
 # 0.20.0 Alpha 3 (2019-08-14)
 

Cargo.toml

@@ -21,7 +21,7 @@ stdweb = ["std_web", "instant/stdweb"]
 [dependencies]
 instant = "0.1"
 lazy_static = "1"
-libc = "0.2"
+libc = "0.2.64"
 log = "0.4"
 serde = { version = "1", optional = true, features = ["serde_derive"] }
 raw-window-handle = "0.3"

src/event_loop.rs

@@ -28,6 +28,7 @@ use crate::{event::Event, monitor::MonitorHandle, platform_impl};
 /// forbidding it), as such it is neither `Send` nor `Sync`. If you need cross-thread access, the
 /// `Window` created from this `EventLoop` _can_ be sent to an other thread, and the
 /// `EventLoopProxy` allows you to wake up an `EventLoop` from another thread.
+///
 pub struct EventLoop<T: 'static> {
     pub(crate) event_loop: platform_impl::EventLoop<T>,
     pub(crate) _marker: ::std::marker::PhantomData<*mut ()>, // Not Send nor Sync
@@ -94,6 +95,18 @@ impl Default for ControlFlow {
 impl EventLoop<()> {
     /// Builds a new event loop with a `()` as the user event type.
     ///
+    /// ***For cross-platform compatibility, the `EventLoop` must be created on the main thread.***
+    /// Attempting to create the event loop on a different thread will panic. This restriction isn't
+    /// strictly necessary on all platforms, but is imposed to eliminate any nasty surprises when
+    /// porting to platforms that require it. `EventLoopExt::new_any_thread` functions are exposed
+    /// in the relevant `platform` module if the target platform supports creating an event loop on
+    /// any thread.
+    ///
+    /// Usage will result in display backend initialisation, this can be controlled on linux
+    /// using an environment variable `WINIT_UNIX_BACKEND`. Legal values are `x11` and `wayland`.
+    /// If it is not set, winit will try to connect to a wayland connection, and if it fails will
+    /// fallback on x11. If this variable is set with any other value, winit will panic.
+    ///
     /// ## Platform-specific
     ///
     /// - **iOS:** Can only be called on the main thread.
@@ -105,10 +118,7 @@ impl EventLoop<()> {
 impl<T> EventLoop<T> {
     /// Builds a new event loop.
     ///
-    /// Usage will result in display backend initialisation, this can be controlled on linux
-    /// using an environment variable `WINIT_UNIX_BACKEND`. Legal values are `x11` and `wayland`.
-    /// If it is not set, winit will try to connect to a wayland connection, and if it fails will
-    /// fallback on x11. If this variable is set with any other value, winit will panic.
+    /// All caveats documented in [`EventLoop::new`] apply to this function.
     ///
     /// ## Platform-specific
     ///

src/platform/unix.rs

@@ -144,35 +144,90 @@ impl<T> EventLoopWindowTargetExtUnix for EventLoopWindowTarget<T> {
 
 /// Additional methods on `EventLoop` that are specific to Unix.
 pub trait EventLoopExtUnix {
-    /// Builds a new `EventLoops` that is forced to use X11.
+    /// Builds a new `EventLoop` that is forced to use X11.
+    ///
+    /// # Panics
+    ///
+    /// If called outside the main thread. To initialize an X11 event loop outside
+    /// the main thread, use [`new_x11_any_thread`](#tymethod.new_x11_any_thread).
     fn new_x11() -> Result<Self, XNotSupported>
     where
         Self: Sized;
 
     /// Builds a new `EventLoop` that is forced to use Wayland.
+    ///
+    /// # Panics
+    ///
+    /// If called outside the main thread. To initialize a Wayland event loop outside
+    /// the main thread, use [`new_wayland_any_thread`](#tymethod.new_wayland_any_thread).
     fn new_wayland() -> Self
     where
         Self: Sized;
+
+    /// Builds a new `EventLoop` on any thread.
+    ///
+    /// This method bypasses the cross-platform compatibility requirement
+    /// that `EventLoop` be created on the main thread.
+    fn new_any_thread() -> Self
+    where
+        Self: Sized;
+
+    /// Builds a new X11 `EventLoop` on any thread.
+    ///
+    /// This method bypasses the cross-platform compatibility requirement
+    /// that `EventLoop` be created on the main thread.
+    fn new_x11_any_thread() -> Result<Self, XNotSupported>
+    where
+        Self: Sized;
+
+    /// Builds a new Wayland `EventLoop` on any thread.
+    ///
+    /// This method bypasses the cross-platform compatibility requirement
+    /// that `EventLoop` be created on the main thread.
+    fn new_wayland_any_thread() -> Self
+    where
+        Self: Sized;
+}
+
+fn wrap_ev<T>(event_loop: LinuxEventLoop<T>) -> EventLoop<T> {
+    EventLoop {
+        event_loop,
+        _marker: std::marker::PhantomData,
+    }
 }
 
 impl<T> EventLoopExtUnix for EventLoop<T> {
+    #[inline]
+    fn new_any_thread() -> Self {
+        wrap_ev(LinuxEventLoop::new_any_thread())
+    }
+
+    #[inline]
+    fn new_x11_any_thread() -> Result<Self, XNotSupported> {
+        LinuxEventLoop::new_x11_any_thread().map(wrap_ev)
+    }
+
+    #[inline]
+    fn new_wayland_any_thread() -> Self {
+        wrap_ev(
+            LinuxEventLoop::new_wayland_any_thread()
+                // TODO: propagate
+                .expect("failed to open Wayland connection"),
+        )
+    }
+
     #[inline]
     fn new_x11() -> Result<Self, XNotSupported> {
-        LinuxEventLoop::new_x11().map(|ev| EventLoop {
-            event_loop: ev,
-            _marker: ::std::marker::PhantomData,
-        })
+        LinuxEventLoop::new_x11().map(wrap_ev)
     }
 
     #[inline]
     fn new_wayland() -> Self {
-        EventLoop {
-            event_loop: match LinuxEventLoop::new_wayland() {
-                Ok(e) => e,
-                Err(_) => panic!(), // TODO: propagate
-            },
-            _marker: ::std::marker::PhantomData,
-        }
+        wrap_ev(
+            LinuxEventLoop::new_wayland()
+                // TODO: propagate
+                .expect("failed to open Wayland connection"),
+        )
     }
 }
 

src/platform/windows.rs

@@ -15,18 +15,52 @@ use crate::{
 
 /// Additional methods on `EventLoop` that are specific to Windows.
 pub trait EventLoopExtWindows {
+    /// Creates an event loop off of the main thread.
+    ///
+    /// # `Window` caveats
+    ///
+    /// Note that any `Window` created on the new thread will be destroyed when the thread
+    /// terminates. Attempting to use a `Window` after its parent thread terminates has
+    /// unspecified, although explicitly not undefined, behavior.
+    fn new_any_thread() -> Self
+    where
+        Self: Sized;
+
     /// By default, winit on Windows will attempt to enable process-wide DPI awareness. If that's
     /// undesirable, you can create an `EventLoop` using this function instead.
     fn new_dpi_unaware() -> Self
     where
         Self: Sized;
+
+    /// Creates a DPI-unaware event loop off of the main thread.
+    ///
+    /// The `Window` caveats in [`new_any_thread`](EventLoopExtWindows::new_any_thread) also apply here.
+    fn new_dpi_unaware_any_thread() -> Self
+    where
+        Self: Sized;
 }
 
 impl<T> EventLoopExtWindows for EventLoop<T> {
+    #[inline]
+    fn new_any_thread() -> Self {
+        EventLoop {
+            event_loop: WindowsEventLoop::new_any_thread(),
+            _marker: ::std::marker::PhantomData,
+        }
+    }
+
     #[inline]
     fn new_dpi_unaware() -> Self {
         EventLoop {
-            event_loop: WindowsEventLoop::with_dpi_awareness(false),
+            event_loop: WindowsEventLoop::new_dpi_unaware(),
+            _marker: ::std::marker::PhantomData,
+        }
+    }
+
+    #[inline]
+    fn new_dpi_unaware_any_thread() -> Self {
+        EventLoop {
+            event_loop: WindowsEventLoop::new_dpi_unaware_any_thread(),
             _marker: ::std::marker::PhantomData,
         }
     }

src/platform_impl/linux/mod.rs

@@ -520,14 +520,22 @@ impl<T: 'static> Clone for EventLoopProxy<T> {
 
 impl<T: 'static> EventLoop<T> {
     pub fn new() -> EventLoop<T> {
+        assert_is_main_thread("new_any_thread");
+
+        EventLoop::new_any_thread()
+    }
+
+    pub fn new_any_thread() -> EventLoop<T> {
         if let Ok(env_var) = env::var(BACKEND_PREFERENCE_ENV_VAR) {
             match env_var.as_str() {
                 "x11" => {
                     // TODO: propagate
-                    return EventLoop::new_x11().expect("Failed to initialize X11 backend");
+                    return EventLoop::new_x11_any_thread()
+                        .expect("Failed to initialize X11 backend");
                 }
                 "wayland" => {
-                    return EventLoop::new_wayland().expect("Failed to initialize Wayland backend");
+                    return EventLoop::new_wayland_any_thread()
+                        .expect("Failed to initialize Wayland backend");
                 }
                 _ => panic!(
                     "Unknown environment variable value for {}, try one of `x11`,`wayland`",
@@ -536,12 +544,12 @@ impl<T: 'static> EventLoop<T> {
             }
         }
 
-        let wayland_err = match EventLoop::new_wayland() {
+        let wayland_err = match EventLoop::new_wayland_any_thread() {
             Ok(event_loop) => return event_loop,
             Err(err) => err,
         };
 
-        let x11_err = match EventLoop::new_x11() {
+        let x11_err = match EventLoop::new_x11_any_thread() {
             Ok(event_loop) => return event_loop,
             Err(err) => err,
         };
@@ -554,10 +562,22 @@ impl<T: 'static> EventLoop<T> {
     }
 
     pub fn new_wayland() -> Result<EventLoop<T>, ConnectError> {
+        assert_is_main_thread("new_wayland_any_thread");
+
+        EventLoop::new_wayland_any_thread()
+    }
+
+    pub fn new_wayland_any_thread() -> Result<EventLoop<T>, ConnectError> {
         wayland::EventLoop::new().map(EventLoop::Wayland)
     }
 
     pub fn new_x11() -> Result<EventLoop<T>, XNotSupported> {
+        assert_is_main_thread("new_x11_any_thread");
+
+        EventLoop::new_x11_any_thread()
+    }
+
+    pub fn new_x11_any_thread() -> Result<EventLoop<T>, XNotSupported> {
         X11_BACKEND
             .lock()
             .as_ref()
@@ -672,3 +692,35 @@ fn sticky_exit_callback<T, F>(
     // user callback
     callback(evt, target, cf)
 }
+
+fn assert_is_main_thread(suggested_method: &str) {
+    if !is_main_thread() {
+        panic!(
+            "Initializing the event loop outside of the main thread is a significant \
+             cross-platform compatibility hazard. If you really, absolutely need to create an \
+             EventLoop on a different thread, please use the `EventLoopExtUnix::{}` function.",
+            suggested_method
+        );
+    }
+}
+
+#[cfg(target_os = "linux")]
+fn is_main_thread() -> bool {
+    use libc::{c_long, getpid, syscall, SYS_gettid};
+
+    unsafe { syscall(SYS_gettid) == getpid() as c_long }
+}
+
+#[cfg(any(target_os = "dragonfly", target_os = "freebsd", target_os = "openbsd"))]
+fn is_main_thread() -> bool {
+    use libc::pthread_main_np;
+
+    unsafe { pthread_main_np() == 1 }
+}
+
+#[cfg(target_os = "netbsd")]
+fn is_main_thread() -> bool {
+    use libc::_lwp_self;
+
+    unsafe { _lwp_self() == 1 }
+}

src/platform_impl/windows/dpi.rs

@@ -43,10 +43,7 @@ lazy_static! {
         get_function!("user32.dll", EnableNonClientDpiScaling);
 }
 
-pub fn become_dpi_aware(enable: bool) {
-    if !enable {
-        return;
-    }
+pub fn become_dpi_aware() {
     static ENABLE_DPI_AWARENESS: Once = Once::new();
     ENABLE_DPI_AWARENESS.call_once(|| {
         unsafe {

src/platform_impl/windows/event_loop.rs

@@ -132,18 +132,40 @@ pub struct EventLoopWindowTarget<T> {
     pub(crate) runner_shared: EventLoopRunnerShared<T>,
 }
 
+macro_rules! main_thread_check {
+    ($fn_name:literal) => {{
+        let thread_id = unsafe { processthreadsapi::GetCurrentThreadId() };
+        if thread_id != main_thread_id() {
+            panic!(concat!(
+                "Initializing the event loop outside of the main thread is a significant \
+                 cross-platform compatibility hazard. If you really, absolutely need to create an \
+                 EventLoop on a different thread, please use the `EventLoopExtWindows::",
+                $fn_name,
+                "` function."
+            ));
+        }
+    }};
+}
+
 impl<T: 'static> EventLoop<T> {
     pub fn new() -> EventLoop<T> {
-        Self::with_dpi_awareness(true)
+        main_thread_check!("new_any_thread");
+
+        Self::new_any_thread()
     }
 
-    pub fn window_target(&self) -> &RootELW<T> {
-        &self.window_target
+    pub fn new_any_thread() -> EventLoop<T> {
+        become_dpi_aware();
+        Self::new_dpi_unaware_any_thread()
     }
 
-    pub fn with_dpi_awareness(dpi_aware: bool) -> EventLoop<T> {
-        become_dpi_aware(dpi_aware);
+    pub fn new_dpi_unaware() -> EventLoop<T> {
+        main_thread_check!("new_dpi_unaware_any_thread");
 
+        Self::new_dpi_unaware_any_thread()
+    }
+
+    pub fn new_dpi_unaware_any_thread() -> EventLoop<T> {
         let thread_id = unsafe { processthreadsapi::GetCurrentThreadId() };
         let runner_shared = Rc::new(ELRShared {
             runner: RefCell::new(None),
@@ -166,6 +188,10 @@ impl<T: 'static> EventLoop<T> {
         }
     }
 
+    pub fn window_target(&self) -> &RootELW<T> {
+        &self.window_target
+    }
+
     pub fn run<F>(mut self, event_handler: F) -> !
     where
         F: 'static + FnMut(Event<T>, &RootELW<T>, &mut ControlFlow),
@@ -178,10 +204,6 @@ impl<T: 'static> EventLoop<T> {
     where
         F: FnMut(Event<T>, &RootELW<T>, &mut ControlFlow),
     {
-        unsafe {
-            winuser::IsGUIThread(1);
-        }
-
         let event_loop_windows_ref = &self.window_target;
 
         let mut runner = unsafe {
@@ -280,6 +302,21 @@ impl<T> EventLoopWindowTarget<T> {
     }
 }
 
+fn main_thread_id() -> DWORD {
+    static mut MAIN_THREAD_ID: DWORD = 0;
+    #[used]
+    #[allow(non_upper_case_globals)]
+    #[link_section = ".CRT$XCU"]
+    static INIT_MAIN_THREAD_ID: unsafe fn() = {
+        unsafe fn initer() {
+            MAIN_THREAD_ID = processthreadsapi::GetCurrentThreadId();
+        }
+        initer
+    };
+
+    unsafe { MAIN_THREAD_ID }
+}
+
 pub(crate) type EventLoopRunnerShared<T> = Rc<ELRShared<T>>;
 pub(crate) struct ELRShared<T> {
     runner: RefCell<Option<EventLoopRunner<T>>>,