From 7ee0318ba648a6012db43c1f367eee24753ec0d6 Mon Sep 17 00:00:00 2001 From: Glenn Watson Date: Fri, 17 Jul 2015 08:07:46 +1000 Subject: [PATCH] Fix a rare crash in some X11 implementations (details below). In some X11 implementations, calling XCloseDisplay from any thread other than the main thread causes heap corruption and an eventual crash. Previously, the window proxy struct held a Weak<> ptr to the display, which was upgraded to a strong arc only during the send of the X message. However, if the main thread closes the window *after* the thread upgrades the weak ref, and this was the last strong reference, then the only remaining strong reference is in the window proxy thread, so the XCloseDisplay is executed on the thread. Instead, use a simpler solution where the display reference used by the window proxy threads is cleared by the main thread when the window is closed, and the XCloseDisplay can only be executed by the main thread. This has the added benefit of not relying on the unstable Weak<> library feature, so we will be able to get this upstreamed. --- src/lib.rs | 2 -- src/x11/window/mod.rs | 37 +++++++++++++++++++++++++++++-------- 2 files changed, 29 insertions(+), 10 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 7a1f1b1b2d..006bf6255b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -23,8 +23,6 @@ //! //! By default only `window` is enabled. -#![feature(arc_weak)] - #[macro_use] extern crate lazy_static; diff --git a/src/x11/window/mod.rs b/src/x11/window/mod.rs index d7cffa2ea7..70be7c489f 100644 --- a/src/x11/window/mod.rs +++ b/src/x11/window/mod.rs @@ -7,7 +7,7 @@ use std::cell::Cell; use std::sync::atomic::AtomicBool; use std::collections::VecDeque; use super::ffi; -use std::sync::{Arc, Mutex, Once, ONCE_INIT, Weak}; +use std::sync::{Arc, Mutex, Once, ONCE_INIT}; use Api; use CursorState; @@ -46,6 +46,13 @@ fn with_c_str(s: &str, f: F) -> T where F: FnOnce(*const libc::c_char) -> f(c_str.as_ptr()) } +struct WindowProxyData { + display: *mut ffi::Display, + window: ffi::Window, +} + +unsafe impl Send for WindowProxyData {} + struct XWindow { display: *mut ffi::Display, window: ffi::Window, @@ -56,6 +63,7 @@ struct XWindow { ic: ffi::XIC, im: ffi::XIM, colormap: ffi::Colormap, + window_proxy_data: Arc>>, } unsafe impl Send for XWindow {} @@ -67,6 +75,10 @@ unsafe impl Sync for Window {} impl Drop for XWindow { fn drop(&mut self) { unsafe { + // Clear out the window proxy data arc, so that any window proxy objects + // are no longer able to send messages to this window. + *self.window_proxy_data.lock().unwrap() = None; + // we don't call MakeCurrent(0, 0) because we are not sure that the context // is still the current one ffi::glx::DestroyContext(self.display as *mut _, self.context); @@ -87,26 +99,28 @@ impl Drop for XWindow { #[derive(Clone)] pub struct WindowProxy { - x: Weak, + data: Arc>>, } impl WindowProxy { pub fn wakeup_event_loop(&self) { - if let Some(x) = self.x.upgrade() { + let window_proxy_data = self.data.lock().unwrap(); + + if let Some(ref data) = *window_proxy_data { let mut xev = ffi::XClientMessageEvent { type_: ffi::ClientMessage, - window: x.window, + window: data.window, format: 32, message_type: 0, serial: 0, send_event: 0, - display: x.display, + display: data.display, data: unsafe { mem::zeroed() }, }; unsafe { - ffi::XSendEvent(x.display, x.window, 0, 0, mem::transmute(&mut xev)); - ffi::XFlush(x.display); + ffi::XSendEvent(data.display, data.window, 0, 0, mem::transmute(&mut xev)); + ffi::XFlush(data.display); } } } @@ -643,6 +657,12 @@ impl Window { } // creating the window object + let window_proxy_data = WindowProxyData { + display: display, + window: window, + }; + let window_proxy_data = Arc::new(Mutex::new(Some(window_proxy_data))); + let window = Window { x: Arc::new(XWindow { display: display, @@ -654,6 +674,7 @@ impl Window { is_fullscreen: builder.monitor.is_some(), xf86_desk_mode: xf86_desk_mode, colormap: cmap, + window_proxy_data: window_proxy_data, }), is_closed: AtomicBool::new(false), wm_delete_window: wm_delete_window, @@ -738,7 +759,7 @@ impl Window { pub fn create_window_proxy(&self) -> WindowProxy { WindowProxy { - x: self.x.downgrade() + data: self.x.window_proxy_data.clone() } }