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

@@ -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));
                 }
             }
 

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<WindowManagerEvent>, Receiver<WindowManagerEvent>) =
-        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(
+        Arc::new(Mutex::new(WindowManager::new(
-            incoming,
+            winevent_listener::event_rx(),
-        )))?))
+        )?))
     };
 
     wm.lock().init()?;

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<Mutex<WindowManager>>) {
-    let receiver = wm.lock().incoming_events.lock().clone();
+    let receiver = wm.lock().incoming_events.clone();
 
     std::thread::spawn(move || {
         tracing::info!("listening");

komorebi/src/static_config.rs

@@ -748,7 +748,7 @@ impl StaticConfig {
     #[allow(clippy::too_many_lines)]
     pub fn preload(
         path: &PathBuf,
-        incoming: Arc<Mutex<Receiver<WindowManagerEvent>>>,
+        incoming: Receiver<WindowManagerEvent>,
     ) -> Result<WindowManager> {
         let content = std::fs::read_to_string(path)?;
         let mut value: Self = serde_json::from_str(&content)?;

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<Monitor>,
     pub monitor_cache: HashMap<usize, Monitor>,
-    pub incoming_events: Arc<Mutex<Receiver<WindowManagerEvent>>>,
+    pub incoming_events: Receiver<WindowManagerEvent>,
     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<Mutex<Receiver<WindowManagerEvent>>>) -> Result<Self> {
+    pub fn new(incoming: Receiver<WindowManagerEvent>) -> Result<Self> {
         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(())

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
+            winevent_listener::event_tx()
-                .lock()
-                .0
                 .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
+            winevent_listener::event_tx()
-                    .lock()
+                .send(event_type)
-                    .0
+                .expect("could not send message on winevent_listener::event_tx");
-                    .send(event_type)
-                    .expect("could not send message on WINEVENT_CALLBACK_CHANNEL");
 
                 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
+            winevent_listener::event_tx()
-                        .lock()
+                .send(event_type)
-                        .0
+                .expect("could not send message on winevent_listener::event_tx");
-                        .send(event_type)
-                        .expect("could not send message on WINEVENT_CALLBACK_CHANNEL");
                 }
                 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
+            winevent_listener::event_tx()
-                        .lock()
+                .send(event_type)
-                        .0
+                .expect("could not send message on winevent_listener::event_tx");
-                        .send(event_type)
-                        .expect("could not send message on WINEVENT_CALLBACK_CHANNEL");
                 }
                 LRESULT(0)
             }

komorebi/src/winevent_listener.rs

@@ -1,105 +1,62 @@
-use std::sync::atomic::AtomicIsize;
+use std::sync::OnceLock;
-use std::sync::atomic::Ordering;
-use std::sync::Arc;
-use std::time::Duration;
 
 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! {
+static CHANNEL: OnceLock<(Sender<WindowManagerEvent>, Receiver<WindowManagerEvent>)> =
-    pub static ref WINEVENT_CALLBACK_CHANNEL: Arc<Mutex<(Sender<WindowManagerEvent>, Receiver<WindowManagerEvent>)>> =
+    OnceLock::new();
-        Arc::new(Mutex::new(crossbeam_channel::unbounded()));
-}
 
-#[derive(Debug, Clone)]
+static EVENT_PUMP: OnceLock<std::thread::JoinHandle<()>> = OnceLock::new();
-pub struct WinEventListener {
-    hook: Arc<AtomicIsize>,
-    outgoing_events: Arc<Mutex<Sender<WindowManagerEvent>>>,
-}
 
-pub fn new(outgoing: Arc<Mutex<Sender<WindowManagerEvent>>>) -> WinEventListener {
+pub fn start() {
-    WinEventListener {
+    EVENT_PUMP.get_or_init(|| {
-        hook: Arc::new(AtomicIsize::new(0)),
+        std::thread::spawn(move || {
-        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<MSG>) -> bool) {
-        Self::start_with_sleep(sleep, cb);
-    }
-
-    fn start_with_sleep(sleep: u64, cb: impl Fn(Option<MSG>) -> bool) {
-        let mut msg: MSG = MSG::default();
-        loop {
-            let mut value: Option<MSG> = None;
             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<WindowManagerEvent>, Receiver<WindowManagerEvent>) {
-            }
+    CHANNEL.get_or_init(crossbeam_channel::unbounded)
-        }
+}
-    }
+
+pub fn event_tx() -> Sender<WindowManagerEvent> {
+    channel().0.clone()
+}
+
+pub fn event_rx() -> Receiver<WindowManagerEvent> {
+    channel().1.clone()
 }