From 0160e8eeeb8419c0c20092468fc6201c9a8d1873 Mon Sep 17 00:00:00 2001 From: James Tucker Date: Sun, 11 Feb 2024 15:47:07 -0800 Subject: [PATCH] fix(wm): cleanup window event messaging - Use a single thread to bind the hook, and then start dispatching. - Use a blocking loop for message dispatching. - Remove the locks around crossbeam channel, as it's already Send + Sync --- komorebi/src/hidden.rs | 2 - komorebi/src/main.rs | 19 ++--- komorebi/src/process_event.rs | 2 +- komorebi/src/static_config.rs | 2 +- komorebi/src/window_manager.rs | 12 +-- komorebi/src/windows_callbacks.rs | 32 +++----- komorebi/src/winevent_listener.rs | 123 ++++++++++-------------------- 7 files changed, 66 insertions(+), 126 deletions(-) diff --git a/komorebi/src/hidden.rs b/komorebi/src/hidden.rs index 47068916..2e35c3b1 100644 --- a/komorebi/src/hidden.rs +++ b/komorebi/src/hidden.rs @@ -1,5 +1,4 @@ use std::sync::atomic::Ordering; -use std::time::Duration; use color_eyre::Result; use windows::core::PCWSTR; @@ -59,7 +58,6 @@ impl Hidden { unsafe { while GetMessageW(&mut message, hidden.hwnd(), 0, 0).into() { DispatchMessageW(&message); - std::thread::sleep(Duration::from_millis(10)); } } diff --git a/komorebi/src/main.rs b/komorebi/src/main.rs index c7b96cf0..06950b93 100644 --- a/komorebi/src/main.rs +++ b/komorebi/src/main.rs @@ -14,8 +14,6 @@ use std::time::Duration; use clap::Parser; use color_eyre::Result; -use crossbeam_channel::Receiver; -use crossbeam_channel::Sender; use crossbeam_utils::Backoff; #[cfg(feature = "deadlock_detection")] use parking_lot::deadlock; @@ -33,7 +31,6 @@ use komorebi::process_event::listen_for_events; use komorebi::process_movement::listen_for_movements; use komorebi::static_config::StaticConfig; use komorebi::window_manager::WindowManager; -use komorebi::window_manager_event::WindowManagerEvent; use komorebi::windows_api::WindowsApi; use komorebi::winevent_listener; use komorebi::CUSTOM_FFM; @@ -183,15 +180,11 @@ fn main() -> Result<()> { WindowsApi::foreground_lock_timeout()?; + winevent_listener::start(); + #[cfg(feature = "deadlock_detection")] detect_deadlocks(); - let (outgoing, incoming): (Sender, Receiver) = - crossbeam_channel::unbounded(); - - let winevent_listener = winevent_listener::new(Arc::new(Mutex::new(outgoing))); - winevent_listener.start(); - Hidden::create("komorebi-hidden")?; let static_config = opts.config.map_or_else( @@ -214,12 +207,12 @@ fn main() -> Result<()> { Arc::new(Mutex::new(StaticConfig::preload( config, - Arc::new(Mutex::new(incoming)), + winevent_listener::event_rx(), )?)) } else { - Arc::new(Mutex::new(WindowManager::new(Arc::new(Mutex::new( - incoming, - )))?)) + Arc::new(Mutex::new(WindowManager::new( + winevent_listener::event_rx(), + )?)) }; wm.lock().init()?; diff --git a/komorebi/src/process_event.rs b/komorebi/src/process_event.rs index f3ac58d7..5adc7dc2 100644 --- a/komorebi/src/process_event.rs +++ b/komorebi/src/process_event.rs @@ -35,7 +35,7 @@ use crate::TRAY_AND_MULTI_WINDOW_IDENTIFIERS; #[tracing::instrument] pub fn listen_for_events(wm: Arc>) { - let receiver = wm.lock().incoming_events.lock().clone(); + let receiver = wm.lock().incoming_events.clone(); std::thread::spawn(move || { tracing::info!("listening"); diff --git a/komorebi/src/static_config.rs b/komorebi/src/static_config.rs index a1fbf77a..bbd0d15c 100644 --- a/komorebi/src/static_config.rs +++ b/komorebi/src/static_config.rs @@ -748,7 +748,7 @@ impl StaticConfig { #[allow(clippy::too_many_lines)] pub fn preload( path: &PathBuf, - incoming: Arc>>, + incoming: Receiver, ) -> Result { let content = std::fs::read_to_string(path)?; let mut value: Self = serde_json::from_str(&content)?; diff --git a/komorebi/src/window_manager.rs b/komorebi/src/window_manager.rs index 78bc5fa9..2027be72 100644 --- a/komorebi/src/window_manager.rs +++ b/komorebi/src/window_manager.rs @@ -45,7 +45,7 @@ use crate::static_config::StaticConfig; use crate::window::Window; use crate::window_manager_event::WindowManagerEvent; use crate::windows_api::WindowsApi; -use crate::winevent_listener::WINEVENT_CALLBACK_CHANNEL; +use crate::winevent_listener; use crate::workspace::Workspace; use crate::BORDER_HWND; use crate::BORDER_OVERFLOW_IDENTIFIERS; @@ -66,7 +66,7 @@ use crate::WORKSPACE_RULES; pub struct WindowManager { pub monitors: Ring, pub monitor_cache: HashMap, - pub incoming_events: Arc>>, + pub incoming_events: Receiver, pub command_listener: UnixListener, pub is_paused: bool, pub invisible_borders: Rect, @@ -168,7 +168,7 @@ impl EnforceWorkspaceRuleOp { impl WindowManager { #[tracing::instrument] - pub fn new(incoming: Arc>>) -> Result { + pub fn new(incoming: Receiver) -> Result { let socket = DATA_DIR.join("komorebi.sock"); match std::fs::remove_file(&socket) { @@ -668,14 +668,14 @@ impl WindowManager { pub fn manage_focused_window(&mut self) -> Result<()> { let hwnd = WindowsApi::foreground_window()?; let event = WindowManagerEvent::Manage(Window { hwnd }); - Ok(WINEVENT_CALLBACK_CHANNEL.lock().0.send(event)?) + Ok(winevent_listener::event_tx().send(event)?) } #[tracing::instrument(skip(self))] pub fn unmanage_focused_window(&mut self) -> Result<()> { let hwnd = WindowsApi::foreground_window()?; let event = WindowManagerEvent::Unmanage(Window { hwnd }); - Ok(WINEVENT_CALLBACK_CHANNEL.lock().0.send(event)?) + Ok(winevent_listener::event_tx().send(event)?) } #[tracing::instrument(skip(self))] @@ -731,7 +731,7 @@ impl WindowManager { if known_hwnd { let event = WindowManagerEvent::Raise(Window { hwnd }); self.has_pending_raise_op = true; - Ok(WINEVENT_CALLBACK_CHANNEL.lock().0.send(event)?) + Ok(winevent_listener::event_tx().send(event)?) } else { tracing::debug!("not raising unknown window: {}", Window { hwnd }); Ok(()) diff --git a/komorebi/src/windows_callbacks.rs b/komorebi/src/windows_callbacks.rs index 7fad8dba..8d15203d 100644 --- a/komorebi/src/windows_callbacks.rs +++ b/komorebi/src/windows_callbacks.rs @@ -37,7 +37,7 @@ use crate::ring::Ring; use crate::window::Window; use crate::window_manager_event::WindowManagerEvent; use crate::windows_api::WindowsApi; -use crate::winevent_listener::WINEVENT_CALLBACK_CHANNEL; +use crate::winevent_listener; use crate::BORDER_COLOUR_CURRENT; use crate::BORDER_RECT; use crate::BORDER_WIDTH; @@ -186,11 +186,9 @@ pub extern "system" fn win_event_hook( if let Ok(should_manage) = window.should_manage(Option::from(event_type)) { if should_manage { - WINEVENT_CALLBACK_CHANNEL - .lock() - .0 + winevent_listener::event_tx() .send(event_type) - .expect("could not send message on WINEVENT_CALLBACK_CHANNEL"); + .expect("could not send message on winevent_listener::event_tx"); } } } @@ -241,11 +239,9 @@ pub extern "system" fn hidden_window( match message { WM_DISPLAYCHANGE => { let event_type = WindowManagerEvent::DisplayChange(Window { hwnd: window.0 }); - WINEVENT_CALLBACK_CHANNEL - .lock() - .0 - .send(event_type) - .expect("could not send message on WINEVENT_CALLBACK_CHANNEL"); + winevent_listener::event_tx() + .send(event_type) + .expect("could not send message on winevent_listener::event_tx"); LRESULT(0) } @@ -256,11 +252,9 @@ pub extern "system" fn hidden_window( || wparam.0 as u32 == SPI_ICONVERTICALSPACING.0 { let event_type = WindowManagerEvent::DisplayChange(Window { hwnd: window.0 }); - WINEVENT_CALLBACK_CHANNEL - .lock() - .0 - .send(event_type) - .expect("could not send message on WINEVENT_CALLBACK_CHANNEL"); + winevent_listener::event_tx() + .send(event_type) + .expect("could not send message on winevent_listener::event_tx"); } LRESULT(0) } @@ -269,11 +263,9 @@ pub extern "system" fn hidden_window( #[allow(clippy::cast_possible_truncation)] if wparam.0 as u32 == DBT_DEVNODES_CHANGED { let event_type = WindowManagerEvent::DisplayChange(Window { hwnd: window.0 }); - WINEVENT_CALLBACK_CHANNEL - .lock() - .0 - .send(event_type) - .expect("could not send message on WINEVENT_CALLBACK_CHANNEL"); + winevent_listener::event_tx() + .send(event_type) + .expect("could not send message on winevent_listener::event_tx"); } LRESULT(0) } diff --git a/komorebi/src/winevent_listener.rs b/komorebi/src/winevent_listener.rs index 896dcf1f..4b9220cf 100644 --- a/komorebi/src/winevent_listener.rs +++ b/komorebi/src/winevent_listener.rs @@ -1,105 +1,62 @@ -use std::sync::atomic::AtomicIsize; -use std::sync::atomic::Ordering; -use std::sync::Arc; -use std::time::Duration; +use std::sync::OnceLock; use crossbeam_channel::Receiver; use crossbeam_channel::Sender; -use lazy_static::lazy_static; -use parking_lot::Mutex; use windows::Win32::Foundation::HWND; use windows::Win32::UI::Accessibility::SetWinEventHook; use windows::Win32::UI::WindowsAndMessaging::DispatchMessageW; -use windows::Win32::UI::WindowsAndMessaging::PeekMessageW; +use windows::Win32::UI::WindowsAndMessaging::GetMessageW; use windows::Win32::UI::WindowsAndMessaging::TranslateMessage; use windows::Win32::UI::WindowsAndMessaging::EVENT_MAX; use windows::Win32::UI::WindowsAndMessaging::EVENT_MIN; use windows::Win32::UI::WindowsAndMessaging::MSG; -use windows::Win32::UI::WindowsAndMessaging::PM_REMOVE; use crate::window_manager_event::WindowManagerEvent; use crate::windows_callbacks; -lazy_static! { - pub static ref WINEVENT_CALLBACK_CHANNEL: Arc, Receiver)>> = - Arc::new(Mutex::new(crossbeam_channel::unbounded())); -} +static CHANNEL: OnceLock<(Sender, Receiver)> = + OnceLock::new(); -#[derive(Debug, Clone)] -pub struct WinEventListener { - hook: Arc, - outgoing_events: Arc>>, -} +static EVENT_PUMP: OnceLock> = OnceLock::new(); -pub fn new(outgoing: Arc>>) -> WinEventListener { - WinEventListener { - hook: Arc::new(AtomicIsize::new(0)), - outgoing_events: outgoing, - } -} - -impl WinEventListener { - pub fn start(self) { - let hook = self.hook.clone(); - let outgoing = self.outgoing_events.lock().clone(); - - std::thread::spawn(move || unsafe { - let hook_ref = SetWinEventHook( - EVENT_MIN, - EVENT_MAX, - None, - Some(windows_callbacks::win_event_hook), - 0, - 0, - 0, - ); - - hook.store(hook_ref.0, Ordering::SeqCst); - - // The code in the callback doesn't work in its own loop, needs to be within - // the MessageLoop callback for the winevent callback to even fire - MessageLoop::start(10, |_msg| { - if let Ok(event) = WINEVENT_CALLBACK_CHANNEL.lock().1.try_recv() { - match outgoing.send(event) { - Ok(()) => {} - Err(error) => { - tracing::error!("{}", error); - } - } - } - - true - }); - }); - } -} - -#[derive(Debug, Copy, Clone)] -pub struct MessageLoop; - -impl MessageLoop { - pub fn start(sleep: u64, cb: impl Fn(Option) -> bool) { - Self::start_with_sleep(sleep, cb); - } - - fn start_with_sleep(sleep: u64, cb: impl Fn(Option) -> bool) { - let mut msg: MSG = MSG::default(); - loop { - let mut value: Option = None; +pub fn start() { + EVENT_PUMP.get_or_init(|| { + std::thread::spawn(move || { unsafe { - if !bool::from(!PeekMessageW(&mut msg, HWND(0), 0, 0, PM_REMOVE)) { + SetWinEventHook( + EVENT_MIN, + EVENT_MAX, + None, + Some(windows_callbacks::win_event_hook), + 0, + 0, + 0, + ) + }; + + loop { + let mut msg: MSG = MSG::default(); + unsafe { + if !GetMessageW(&mut msg, HWND(0), 0, 0).as_bool() { + tracing::info!("windows event processing shutdown"); + break; + }; TranslateMessage(&msg); DispatchMessageW(&msg); - - value = Some(msg); } } - - std::thread::sleep(Duration::from_millis(sleep)); - - if !cb(value) { - break; - } - } - } + }) + }); +} + +fn channel() -> &'static (Sender, Receiver) { + CHANNEL.get_or_init(crossbeam_channel::unbounded) +} + +pub fn event_tx() -> Sender { + channel().0.clone() +} + +pub fn event_rx() -> Receiver { + channel().1.clone() }