fix(wm): restart bm and wsr notif handlers on fail

This commit ensures that if and when either the border_manager or
workspace_reconciliator notification handlers crash in their respective
threads, they will be restarted instead of killing the whole thread.

This is acheived via a loop running in the spawned threads of both
listeners, which will report whichever error killed the notification
handler functions.

Clippy annotations to deny expect and unwrap calls have been added to
these two modules.

Calls to DestroyWindow for expired borders were sporadically failing in
unpredictable circumstances, so these have been switched out with calls
to CloseWindow which has been working well so far with the stackbar
feature.

komorebi/src/border_manager/border.rs

@@ -60,6 +60,7 @@ pub extern "system" fn border_hwnds(hwnd: HWND, lparam: LPARAM) -> BOOL {
     true.into()
 }
 
+#[derive(Debug)]
 pub struct Border {
     pub hwnd: isize,
 }
@@ -116,7 +117,7 @@ impl Border {
     }
 
     pub fn destroy(&self) -> color_eyre::Result<()> {
-        WindowsApi::destroy_window(self.hwnd())
+        WindowsApi::close_window(self.hwnd())
     }
 
     pub fn update(&self, rect: &Rect) -> color_eyre::Result<()> {

komorebi/src/border_manager/mod.rs

@@ -1,3 +1,5 @@
+#![deny(clippy::unwrap_used, clippy::expect_used)]
+
 mod border;
 
 use crossbeam_channel::Receiver;
@@ -9,6 +11,7 @@ use parking_lot::Mutex;
 use schemars::JsonSchema;
 use serde::Deserialize;
 use serde::Serialize;
+use std::collections::hash_map::Entry;
 use std::collections::HashMap;
 use std::sync::atomic::AtomicBool;
 use std::sync::atomic::AtomicI32;
@@ -69,6 +72,11 @@ pub fn event_rx() -> Receiver<Notification> {
 
 pub fn destroy_all_borders() -> color_eyre::Result<()> {
     let mut borders = BORDER_STATE.lock();
+    tracing::info!(
+        "purging known borders: {:?}",
+        borders.iter().map(|b| b.1.hwnd).collect::<Vec<_>>()
+    );
+
     for (_, border) in borders.iter() {
         border.destroy()?;
     }
@@ -85,128 +93,64 @@ pub fn destroy_all_borders() -> color_eyre::Result<()> {
         &mut remaining_hwnds as *mut Vec<isize> as isize,
     )?;
 
-    for hwnd in remaining_hwnds {
-        Border::from(hwnd).destroy()?;
+    if !remaining_hwnds.is_empty() {
+        tracing::info!("purging unknown borders: {:?}", remaining_hwnds);
+
+        for hwnd in remaining_hwnds {
+            Border::from(hwnd).destroy()?;
+        }
     }
 
     Ok(())
 }
 
 pub fn listen_for_notifications(wm: Arc<Mutex<WindowManager>>) {
+    std::thread::spawn(move || loop {
+        match handle_notifications(wm.clone()) {
+            Ok(()) => {
+                tracing::warn!("restarting finished thread");
+            }
+            Err(error) => {
+                tracing::warn!("restarting failed thread: {}", error);
+            }
+        }
+    });
+}
+
+pub fn handle_notifications(wm: Arc<Mutex<WindowManager>>) -> color_eyre::Result<()> {
     tracing::info!("listening");
+
     let receiver = event_rx();
 
-    std::thread::spawn(move || -> color_eyre::Result<()> {
-        'receiver: for _ in receiver {
-            let mut borders = BORDER_STATE.lock();
-            let mut borders_monitors = BORDERS_MONITORS.lock();
+    'receiver: for _ in receiver {
+        let mut borders = BORDER_STATE.lock();
+        let mut borders_monitors = BORDERS_MONITORS.lock();
 
-            // Check the wm state every time we receive a notification
-            let state = wm.lock();
+        // Check the wm state every time we receive a notification
+        let state = wm.lock();
 
-            if !BORDER_ENABLED.load_consume() || state.is_paused {
-                if !borders.is_empty() {
-                    for (_, border) in borders.iter() {
-                        border.destroy()?;
-                    }
-
-                    borders.clear();
+        if !BORDER_ENABLED.load_consume() || state.is_paused {
+            if !borders.is_empty() {
+                for (_, border) in borders.iter() {
+                    border.destroy()?;
                 }
 
-                continue 'receiver;
+                borders.clear();
             }
 
-            let focused_monitor_idx = state.focused_monitor_idx();
+            continue 'receiver;
+        }
 
-            for (monitor_idx, m) in state.monitors.elements().iter().enumerate() {
-                // Only operate on the focused workspace of each monitor
-                if let Some(ws) = m.focused_workspace() {
-                    // Workspaces with tiling disabled don't have borders
-                    if !ws.tile() {
-                        let mut to_remove = vec![];
-                        for (id, border) in borders.iter() {
-                            if borders_monitors.get(id).copied().unwrap_or_default() == monitor_idx
-                            {
-                                border.destroy()?;
-                                to_remove.push(id.clone());
-                            }
-                        }
-
-                        for id in &to_remove {
-                            borders.remove(id);
-                        }
-
-                        continue 'receiver;
-                    }
-
-                    // Handle the monocle container separately
-                    if let Some(monocle) = ws.monocle_container() {
-                        let mut to_remove = vec![];
-                        for (id, border) in borders.iter() {
-                            if borders_monitors.get(id).copied().unwrap_or_default() == monitor_idx
-                            {
-                                border.destroy()?;
-                                to_remove.push(id.clone());
-                            }
-                        }
-
-                        for id in &to_remove {
-                            borders.remove(id);
-                        }
-
-                        let border = borders.entry(monocle.id().clone()).or_insert_with(|| {
-                            Border::create(monocle.id()).expect("border creation failed")
-                        });
-
-                        borders_monitors.insert(monocle.id().clone(), monitor_idx);
-
-                        {
-                            let mut focus_state = FOCUS_STATE.lock();
-                            focus_state.insert(border.hwnd, WindowKind::Monocle);
-                        }
-
-                        let rect = WindowsApi::window_rect(
-                            monocle
-                                .focused_window()
-                                .expect("monocle container has no focused window")
-                                .hwnd(),
-                        )?;
-
-                        border.update(&rect)?;
-                        continue 'receiver;
-                    }
-
-                    let is_maximized = WindowsApi::is_zoomed(HWND(
-                        WindowsApi::foreground_window().unwrap_or_default(),
-                    ));
-
-                    if is_maximized {
-                        let mut to_remove = vec![];
-                        for (id, border) in borders.iter() {
-                            if borders_monitors.get(id).copied().unwrap_or_default() == monitor_idx
-                            {
-                                border.destroy()?;
-                                to_remove.push(id.clone());
-                            }
-                        }
-
-                        for id in &to_remove {
-                            borders.remove(id);
-                        }
-
-                        continue 'receiver;
-                    }
-
-                    // Destroy any borders not associated with the focused workspace
-                    let container_ids = ws
-                        .containers()
-                        .iter()
-                        .map(|c| c.id().clone())
-                        .collect::<Vec<_>>();
+        let focused_monitor_idx = state.focused_monitor_idx();
 
+        for (monitor_idx, m) in state.monitors.elements().iter().enumerate() {
+            // Only operate on the focused workspace of each monitor
+            if let Some(ws) = m.focused_workspace() {
+                // Workspaces with tiling disabled don't have borders
+                if !ws.tile() {
                     let mut to_remove = vec![];
                     for (id, border) in borders.iter() {
-                        if borders_monitors.get(id).copied().unwrap_or_default() == monitor_idx && !container_ids.contains(id) {
+                        if borders_monitors.get(id).copied().unwrap_or_default() == monitor_idx {
                             border.destroy()?;
                             to_remove.push(id.clone());
                         }
@@ -216,78 +160,169 @@ pub fn listen_for_notifications(wm: Arc<Mutex<WindowManager>>) {
                         borders.remove(id);
                     }
 
-                    for (idx, c) in ws.containers().iter().enumerate() {
-                        // Update border when moving or resizing with mouse
-                        if state.pending_move_op.is_some() && idx == ws.focused_container_idx() {
-                            let restore_z_order = *Z_ORDER.lock();
-                            *Z_ORDER.lock() = ZOrder::TopMost;
+                    continue 'receiver;
+                }
 
-                            let mut rect = WindowsApi::window_rect(
-                                c.focused_window()
-                                    .expect("container has no focused window")
-                                    .hwnd(),
-                            )?;
+                // Handle the monocle container separately
+                if let Some(monocle) = ws.monocle_container() {
+                    let mut to_remove = vec![];
+                    for (id, border) in borders.iter() {
+                        if borders_monitors.get(id).copied().unwrap_or_default() == monitor_idx {
+                            border.destroy()?;
+                            to_remove.push(id.clone());
+                        }
+                    }
 
-                            while WindowsApi::lbutton_is_pressed() {
-                                let border = borders.entry(c.id().clone()).or_insert_with(|| {
-                                    Border::create(c.id()).expect("border creation failed")
-                                });
+                    for id in &to_remove {
+                        borders.remove(id);
+                    }
 
-                                let new_rect = WindowsApi::window_rect(
-                                    c.focused_window()
-                                        .expect("container has no focused window")
-                                        .hwnd(),
-                                )?;
-
-                                if rect != new_rect {
-                                    rect = new_rect;
-                                    border.update(&rect)?;
-                                }
+                    let border = match borders.entry(monocle.id().clone()) {
+                        Entry::Occupied(entry) => entry.into_mut(),
+                        Entry::Vacant(entry) => {
+                            if let Ok(border) = Border::create(monocle.id()) {
+                                entry.insert(border)
+                            } else {
+                                continue 'receiver;
                             }
-
-                            *Z_ORDER.lock() = restore_z_order;
-
-                            continue 'receiver;
                         }
+                    };
 
-                        // Get the border entry for this container from the map or create one
-                        let border = borders.entry(c.id().clone()).or_insert_with(|| {
-                            Border::create(c.id()).expect("border creation failed")
-                        });
+                    borders_monitors.insert(monocle.id().clone(), monitor_idx);
 
-                        borders_monitors.insert(c.id().clone(), monitor_idx);
+                    {
+                        let mut focus_state = FOCUS_STATE.lock();
+                        focus_state.insert(border.hwnd, WindowKind::Monocle);
+                    }
 
-                        // Update the focused state for all containers on this workspace
-                        {
-                            let mut focus_state = FOCUS_STATE.lock();
-                            focus_state.insert(
-                                border.hwnd,
-                                if idx != ws.focused_container_idx()
-                                    || monitor_idx != focused_monitor_idx
-                                {
-                                    WindowKind::Unfocused
-                                } else if c.windows().len() > 1 {
-                                    WindowKind::Stack
-                                } else {
-                                    WindowKind::Single
-                                },
-                            );
+                    let rect = WindowsApi::window_rect(
+                        monocle.focused_window().copied().unwrap_or_default().hwnd(),
+                    )?;
+
+                    border.update(&rect)?;
+                    continue 'receiver;
+                }
+
+                let is_maximized = WindowsApi::is_zoomed(HWND(
+                    WindowsApi::foreground_window().unwrap_or_default(),
+                ));
+
+                if is_maximized {
+                    let mut to_remove = vec![];
+                    for (id, border) in borders.iter() {
+                        if borders_monitors.get(id).copied().unwrap_or_default() == monitor_idx {
+                            border.destroy()?;
+                            to_remove.push(id.clone());
                         }
+                    }
 
-                        let rect = WindowsApi::window_rect(
-                            c.focused_window()
-                                .expect("container has no focused window")
-                                .hwnd(),
+                    for id in &to_remove {
+                        borders.remove(id);
+                    }
+
+                    continue 'receiver;
+                }
+
+                // Destroy any borders not associated with the focused workspace
+                let container_ids = ws
+                    .containers()
+                    .iter()
+                    .map(|c| c.id().clone())
+                    .collect::<Vec<_>>();
+
+                let mut to_remove = vec![];
+                for (id, border) in borders.iter() {
+                    if borders_monitors.get(id).copied().unwrap_or_default() == monitor_idx
+                        && !container_ids.contains(id)
+                    {
+                        border.destroy()?;
+                        to_remove.push(id.clone());
+                    }
+                }
+
+                for id in &to_remove {
+                    borders.remove(id);
+                }
+
+                for (idx, c) in ws.containers().iter().enumerate() {
+                    // Update border when moving or resizing with mouse
+                    if state.pending_move_op.is_some() && idx == ws.focused_container_idx() {
+                        let restore_z_order = *Z_ORDER.lock();
+                        *Z_ORDER.lock() = ZOrder::TopMost;
+
+                        let mut rect = WindowsApi::window_rect(
+                            c.focused_window().copied().unwrap_or_default().hwnd(),
                         )?;
 
-                        border.update(&rect)?;
+                        while WindowsApi::lbutton_is_pressed() {
+                            let border = match borders.entry(c.id().clone()) {
+                                Entry::Occupied(entry) => entry.into_mut(),
+                                Entry::Vacant(entry) => {
+                                    if let Ok(border) = Border::create(c.id()) {
+                                        entry.insert(border)
+                                    } else {
+                                        continue 'receiver;
+                                    }
+                                }
+                            };
+
+                            let new_rect = WindowsApi::window_rect(
+                                c.focused_window().copied().unwrap_or_default().hwnd(),
+                            )?;
+
+                            if rect != new_rect {
+                                rect = new_rect;
+                                border.update(&rect)?;
+                            }
+                        }
+
+                        *Z_ORDER.lock() = restore_z_order;
+
+                        continue 'receiver;
                     }
+
+                    // Get the border entry for this container from the map or create one
+                    let border = match borders.entry(c.id().clone()) {
+                        Entry::Occupied(entry) => entry.into_mut(),
+                        Entry::Vacant(entry) => {
+                            if let Ok(border) = Border::create(c.id()) {
+                                entry.insert(border)
+                            } else {
+                                continue 'receiver;
+                            }
+                        }
+                    };
+
+                    borders_monitors.insert(c.id().clone(), monitor_idx);
+
+                    // Update the focused state for all containers on this workspace
+                    {
+                        let mut focus_state = FOCUS_STATE.lock();
+                        focus_state.insert(
+                            border.hwnd,
+                            if idx != ws.focused_container_idx()
+                                || monitor_idx != focused_monitor_idx
+                            {
+                                WindowKind::Unfocused
+                            } else if c.windows().len() > 1 {
+                                WindowKind::Stack
+                            } else {
+                                WindowKind::Single
+                            },
+                        );
+                    }
+
+                    let rect = WindowsApi::window_rect(
+                        c.focused_window().copied().unwrap_or_default().hwnd(),
+                    )?;
+
+                    border.update(&rect)?;
                 }
             }
         }
+    }
 
-        Ok(())
-    });
+    Ok(())
 }
 
 #[derive(Debug, Copy, Clone, Serialize, Deserialize, JsonSchema)]

komorebi/src/window.rs

@@ -38,7 +38,7 @@ use crate::PERMAIGNORE_CLASSES;
 use crate::REGEX_IDENTIFIERS;
 use crate::WSL2_UI_PROCESSES;
 
-#[derive(Debug, Clone, Copy, Deserialize, JsonSchema)]
+#[derive(Debug, Default, Clone, Copy, Deserialize, JsonSchema)]
 pub struct Window {
     pub hwnd: isize,
 }

komorebi/src/windows_api.rs

@@ -121,7 +121,6 @@ use windows::Win32::UI::WindowsAndMessaging::SYSTEM_PARAMETERS_INFO_ACTION;
 use windows::Win32::UI::WindowsAndMessaging::SYSTEM_PARAMETERS_INFO_UPDATE_FLAGS;
 use windows::Win32::UI::WindowsAndMessaging::WINDOW_LONG_PTR_INDEX;
 use windows::Win32::UI::WindowsAndMessaging::WM_CLOSE;
-use windows::Win32::UI::WindowsAndMessaging::WM_DESTROY;
 use windows::Win32::UI::WindowsAndMessaging::WNDCLASSW;
 use windows::Win32::UI::WindowsAndMessaging::WNDENUMPROC;
 use windows::Win32::UI::WindowsAndMessaging::WS_DISABLED;
@@ -433,13 +432,6 @@ impl WindowsApi {
         }
     }
 
-    pub fn destroy_window(hwnd: HWND) -> Result<()> {
-        match Self::post_message(hwnd, WM_DESTROY, WPARAM(0), LPARAM(0)) {
-            Ok(()) => Ok(()),
-            Err(_) => Err(anyhow!("could not close window")),
-        }
-    }
-
     pub fn hide_window(hwnd: HWND) {
         Self::show_window(hwnd, SW_HIDE);
     }

komorebi/src/workspace_reconciliator.rs

@@ -1,3 +1,5 @@
+#![deny(clippy::unwrap_used, clippy::expect_used)]
+
 use crate::WindowManager;
 use crossbeam_channel::Receiver;
 use crossbeam_channel::Sender;
@@ -26,31 +28,42 @@ pub fn event_rx() -> Receiver<Notification> {
 }
 
 pub fn listen_for_notifications(wm: Arc<Mutex<WindowManager>>) {
-    tracing::info!("listening");
-    let receiver = event_rx();
-
-    std::thread::spawn(move || -> color_eyre::Result<()> {
-        for notification in receiver {
-            tracing::info!("running reconciliation");
-            let mut wm = wm.lock();
-            let focused_monitor_idx = wm.focused_monitor_idx();
-            let focused_workspace_idx =
-                wm.focused_workspace_idx_for_monitor_idx(focused_monitor_idx)?;
-
-            let focused_pair = (focused_monitor_idx, focused_workspace_idx);
-            let updated_pair = (notification.monitor_idx, notification.workspace_idx);
-
-            if focused_pair != updated_pair {
-                wm.focus_monitor(notification.monitor_idx)?;
-                let mouse_follows_focus = wm.mouse_follows_focus;
-
-                if let Some(monitor) = wm.focused_monitor_mut() {
-                    monitor.focus_workspace(notification.workspace_idx)?;
-                    monitor.load_focused_workspace(mouse_follows_focus)?;
-                }
+    std::thread::spawn(move || loop {
+        match handle_notifications(wm.clone()) {
+            Ok(()) => {
+                tracing::warn!("restarting finished thread");
+            }
+            Err(error) => {
+                tracing::warn!("restarting failed thread: {}", error);
             }
         }
-
-        Ok(())
     });
 }
+pub fn handle_notifications(wm: Arc<Mutex<WindowManager>>) -> color_eyre::Result<()> {
+    tracing::info!("listening");
+
+    let receiver = event_rx();
+
+    for notification in receiver {
+        tracing::info!("running reconciliation");
+        let mut wm = wm.lock();
+        let focused_monitor_idx = wm.focused_monitor_idx();
+        let focused_workspace_idx =
+            wm.focused_workspace_idx_for_monitor_idx(focused_monitor_idx)?;
+
+        let focused_pair = (focused_monitor_idx, focused_workspace_idx);
+        let updated_pair = (notification.monitor_idx, notification.workspace_idx);
+
+        if focused_pair != updated_pair {
+            wm.focus_monitor(notification.monitor_idx)?;
+            let mouse_follows_focus = wm.mouse_follows_focus;
+
+            if let Some(monitor) = wm.focused_monitor_mut() {
+                monitor.focus_workspace(notification.workspace_idx)?;
+                monitor.load_focused_workspace(mouse_follows_focus)?;
+            }
+        }
+    }
+
+    Ok(())
+}