vtimofei
/
smithay
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

x11: Do not hold lock when dispatching events

This commit is contained in:
i509VCB 2021-12-06 15:31:55 -06:00
parent 6b1788d002
commit 09ce3da742
1 changed files with 181 additions and 203 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

140
src/backend/x11/mod.rs
View File

@ -249,6 +249,7 @@ impl X11Backend {
pub fn handle(&self) -> X11Handle { pub fn handle(&self) -> X11Handle {
X11Handle { X11Handle {
log: self.log.clone(), log: self.log.clone(),
connection: self.connection.clone(),
inner: self.inner.clone(), inner: self.inner.clone(),
} }
} }
@ -356,6 +357,7 @@ fn dri3_init(x11: &X11Inner) -> Result<DrmNode, X11Error> {
#[derive(Debug)] #[derive(Debug)]
pub struct X11Handle { pub struct X11Handle {
log: Logger, log: Logger,
connection: Arc<RustConnection>,
inner: Arc<Mutex<X11Inner>>, inner: Arc<Mutex<X11Inner>>,
} }
@ -367,7 +369,7 @@ impl X11Handle {
/// Returns the underlying connection to the X server. /// Returns the underlying connection to the X server.
pub fn connection(&self) -> Arc<RustConnection> { pub fn connection(&self) -> Arc<RustConnection> {
self.inner.lock().unwrap().connection.clone() self.connection.clone()
} }
/// Returns the format of the window. /// Returns the format of the window.
@ -709,11 +711,11 @@ impl EventSource for X11Backend {
F: FnMut(Self::Event, &mut Self::Metadata) -> Self::Ret, F: FnMut(Self::Event, &mut Self::Metadata) -> Self::Ret,
{ {
let connection = self.connection.clone(); let connection = self.connection.clone();
let log = self.log.clone();
let inner = self.inner.clone(); let inner = self.inner.clone();
let mut inner = inner.lock().unwrap();
let post_action = self.source.process_events(readiness, token, |event, _| { let post_action = self.source.process_events(readiness, token, |event, _| {
inner.process_event(event, &mut callback); X11Inner::process_event(&inner, &log, event, &mut callback);
})?; })?;
// Flush the connection so changes to the window state during callbacks can be emitted. // Flush the connection so changes to the window state during callbacks can be emitted.
@ -751,19 +753,21 @@ pub(crate) struct X11Inner {
} }
impl X11Inner { impl X11Inner {
pub fn process_event<F>(&mut self, event: x11::Event, callback: &mut F) fn process_event<F>(inner: &Arc<Mutex<X11Inner>>, log: &Logger, event: x11::Event, callback: &mut F)
where where
F: FnMut(X11Event, &mut Window), F: FnMut(X11Event, &mut Window),
{ {
use self::X11Event::Input; use self::X11Event::Input;
match event { fn window_from_id(inner: &Arc<Mutex<X11Inner>>, id: &u32) -> Option<Arc<WindowInner>> {
x11::Event::ButtonPress(button_press) => { inner.lock().unwrap().windows.get(id).cloned()
if !self.windows.contains_key(&button_press.event) {
return;
} }
let window = self.windows.get(&button_press.event).unwrap(); // If X11 is deadlocking somewhere here, make sure you drop your mutex guards.
match event {
x11::Event::ButtonPress(button_press) => {
if let Some(window) = window_from_id(inner, &button_press.event) {
// X11 decided to associate scroll wheel with a button, 4, 5, 6 and 7 for // X11 decided to associate scroll wheel with a button, 4, 5, 6 and 7 for
// up, down, right and left. For scrolling, a press event is emitted and a // up, down, right and left. For scrolling, a press event is emitted and a
// release is them immediately followed for scrolling. This means we can // release is them immediately followed for scrolling. This means we can
@ -808,7 +812,7 @@ impl X11Inner {
}, },
}, },
}), }),
&mut Window(Arc::downgrade(window)), &mut Window(Arc::downgrade(&window)),
) )
} else { } else {
callback( callback(
@ -819,18 +823,13 @@ impl X11Inner {
state: ButtonState::Pressed, state: ButtonState::Pressed,
}, },
}), }),
&mut Window(Arc::downgrade(window)), &mut Window(Arc::downgrade(&window)),
) )
} }
} }
}
x11::Event::ButtonRelease(button_release) => { x11::Event::ButtonRelease(button_release) => {
if !self.windows.contains_key(&button_release.event) {
return;
}
let window = self.windows.get(&button_release.event).unwrap();
// Ignore release tick because this event is always sent immediately after the press // Ignore release tick because this event is always sent immediately after the press
// tick for scrolling and the backend will dispatch release event automatically during // tick for scrolling and the backend will dispatch release event automatically during
// the press event. // the press event.
@ -838,6 +837,7 @@ impl X11Inner {
return; return;
} }
if let Some(window) = window_from_id(inner, &button_release.event) {
callback( callback(
Input(InputEvent::PointerButton { Input(InputEvent::PointerButton {
event: X11MouseInputEvent { event: X11MouseInputEvent {
@ -846,14 +846,15 @@ impl X11Inner {
state: ButtonState::Released, state: ButtonState::Released,
}, },
}), }),
&mut Window(Arc::downgrade(window)), &mut Window(Arc::downgrade(&window)),
); );
} }
}
x11::Event::KeyPress(key_press) => { x11::Event::KeyPress(key_press) => {
if !self.windows.contains_key(&key_press.event) { if let Some(window) = window_from_id(inner, &key_press.event) {
return; // Do not hold the lock.
} let count = { inner.lock().unwrap().key_counter.fetch_add(1, Ordering::SeqCst) + 1 };
callback( callback(
Input(InputEvent::Keyboard { Input(InputEvent::Keyboard {
@ -865,23 +866,27 @@ impl X11Inner {
// //
// https://github.com/freedesktop/xorg-xf86-input-libinput/blob/master/src/xf86libinput.c#L54 // https://github.com/freedesktop/xorg-xf86-input-libinput/blob/master/src/xf86libinput.c#L54
key: key_press.detail as u32 - 8, key: key_press.detail as u32 - 8,
count: self.key_counter.fetch_add(1, Ordering::SeqCst) + 1, count,
state: KeyState::Pressed, state: KeyState::Pressed,
}, },
}), }),
&mut Window(Arc::downgrade(self.windows.get(&key_press.event).unwrap())), &mut Window(Arc::downgrade(&window)),
) )
} }
}
x11::Event::KeyRelease(key_release) => { x11::Event::KeyRelease(key_release) => {
if !self.windows.contains_key(&key_release.event) { if let Some(window) = window_from_id(inner, &key_release.event) {
return; let count = {
} let key_counter = inner.lock().unwrap().key_counter.clone();
// atomic u32 has no checked_sub, so load and store to do the same. // atomic u32 has no checked_sub, so load and store to do the same.
let mut key_counter_val = self.key_counter.load(Ordering::SeqCst); let mut count = key_counter.load(Ordering::SeqCst);
key_counter_val = key_counter_val.saturating_sub(1); count = count.saturating_sub(1);
self.key_counter.store(key_counter_val, Ordering::SeqCst); key_counter.store(count, Ordering::SeqCst);
count
};
callback( callback(
Input(InputEvent::Keyboard { Input(InputEvent::Keyboard {
@ -893,21 +898,17 @@ impl X11Inner {
// //
// https://github.com/freedesktop/xorg-xf86-input-libinput/blob/master/src/xf86libinput.c#L54 // https://github.com/freedesktop/xorg-xf86-input-libinput/blob/master/src/xf86libinput.c#L54
key: key_release.detail as u32 - 8, key: key_release.detail as u32 - 8,
count: key_counter_val, count,
state: KeyState::Released, state: KeyState::Released,
}, },
}), }),
&mut Window(Arc::downgrade(self.windows.get(&key_release.event).unwrap())), &mut Window(Arc::downgrade(&window)),
); );
} }
}
x11::Event::MotionNotify(motion_notify) => { x11::Event::MotionNotify(motion_notify) => {
if !self.windows.contains_key(&motion_notify.event) { if let Some(window) = window_from_id(inner, &motion_notify.event) {
return;
}
let window = self.windows.get(&motion_notify.event).unwrap();
// Use event_x/y since those are relative the the window receiving events. // Use event_x/y since those are relative the the window receiving events.
let x = motion_notify.event_x as f64; let x = motion_notify.event_x as f64;
let y = motion_notify.event_y as f64; let y = motion_notify.event_y as f64;
@ -923,17 +924,13 @@ impl X11Inner {
size: window_size, size: window_size,
}, },
}), }),
&mut Window(Arc::downgrade(window)), &mut Window(Arc::downgrade(&window)),
) )
} }
}
x11::Event::ConfigureNotify(configure_notify) => { x11::Event::ConfigureNotify(configure_notify) => {
if !self.windows.contains_key(&configure_notify.window) { if let Some(window) = window_from_id(inner, &configure_notify.window) {
return;
}
let window = self.windows.get(&configure_notify.window).unwrap();
let previous_size = { *window.size.lock().unwrap() }; let previous_size = { *window.size.lock().unwrap() };
// Did the size of the window change? // Did the size of the window change?
@ -950,7 +947,7 @@ impl X11Inner {
(callback)( (callback)(
X11Event::Resized(configure_notify_size), X11Event::Resized(configure_notify_size),
&mut Window(Arc::downgrade(window)), &mut Window(Arc::downgrade(&window)),
); );
if let Some(resize_sender) = &*window.resize.lock().unwrap() { if let Some(resize_sender) = &*window.resize.lock().unwrap() {
@ -958,63 +955,44 @@ impl X11Inner {
} }
} }
} }
x11::Event::EnterNotify(enter_notify) => {
if !self.windows.contains_key(&enter_notify.event) {
return;
} }
self.windows.get(&enter_notify.event).unwrap().cursor_enter(); x11::Event::EnterNotify(enter_notify) => {
if let Some(window) = window_from_id(inner, &enter_notify.event) {
window.cursor_enter();
}
} }
x11::Event::LeaveNotify(leave_notify) => { x11::Event::LeaveNotify(leave_notify) => {
if !self.windows.contains_key(&leave_notify.event) { if let Some(window) = window_from_id(inner, &leave_notify.event) {
return; window.cursor_leave();
} }
self.windows.get(&leave_notify.event).unwrap().cursor_leave();
} }
x11::Event::ClientMessage(client_message) => { x11::Event::ClientMessage(client_message) => {
if !self.windows.contains_key(&client_message.window) { if let Some(window) = window_from_id(inner, &client_message.window) {
return;
}
let window = self.windows.get(&client_message.window).unwrap();
if client_message.data.as_data32()[0] == window.atoms.WM_DELETE_WINDOW if client_message.data.as_data32()[0] == window.atoms.WM_DELETE_WINDOW
// Destroy the window? // Destroy the window?
{ {
(callback)(X11Event::CloseRequested, &mut Window(Arc::downgrade(window))); (callback)(X11Event::CloseRequested, &mut Window(Arc::downgrade(&window)));
}
} }
} }
x11::Event::Expose(expose) => { x11::Event::Expose(expose) => {
if !self.windows.contains_key(&expose.window) { if let Some(window) = window_from_id(inner, &expose.window) {
return;
}
if expose.count == 0 { if expose.count == 0 {
(callback)( (callback)(X11Event::Refresh, &mut Window(Arc::downgrade(&window)));
X11Event::Refresh, }
&mut Window(Arc::downgrade(self.windows.get(&expose.window).unwrap())),
);
} }
} }
x11::Event::PresentCompleteNotify(complete_notify) => { x11::Event::PresentCompleteNotify(complete_notify) => {
if !self.windows.contains_key(&complete_notify.window) { if let Some(window) = window_from_id(inner, &complete_notify.window) {
return;
}
let window = self.windows.get(&complete_notify.window).unwrap();
window.last_msc.store(complete_notify.msc, Ordering::SeqCst); window.last_msc.store(complete_notify.msc, Ordering::SeqCst);
(callback)( (callback)(X11Event::PresentCompleted, &mut Window(Arc::downgrade(&window)));
X11Event::PresentCompleted, }
&mut Window(Arc::downgrade(self.windows.get(&complete_notify.window).unwrap())),
);
} }
x11::Event::PresentIdleNotify(_) => { x11::Event::PresentIdleNotify(_) => {
@ -1022,7 +1000,7 @@ impl X11Inner {
} }
x11::Event::Error(e) => { x11::Event::Error(e) => {
error!(self.log, "X11 protocol error: {:?}", e); error!(log, "X11 protocol error: {:?}", e);
} }
_ => (), _ => (),