Skip to content

Commit

Permalink
fix(reaper): avoid deadlocks at startup
Browse files Browse the repository at this point in the history
Previously the reaper at startup would lock it's own `HWNDS_CACHE` and
then try to lock the WM to get its `known_hwnds`.

However if there was an event in the meantime, process_event would lock
the WM first and then it would try to lock the reaper's `HWNDS_CACHE` to
update it.  This would deadlock since that would be locked by the reaper
waiting for the WM lock to be released.

This commit now makes it so we pass the `known_hwnds` to the reaper as
an argument at startup and it also rearranges the order of loading the
listeners.

Now komorebi first loads all the manager-type listeners, and only
afterwards does it load the commands and events listeners.

After some testing this seems to be the best order that doesn't cause
any issues at all!

There were some other issues that I've noticed before when starting
komorebi while having other 3rd parties trying to subscribe to it (like
komorebi-bar and YASB), which would make those subscribers lock the
`process_command` thread. This doesn't seem to be happening on my tests
anymore with this new order.
  • Loading branch information
alex-ds13 authored and LGUG2Z committed Feb 24, 2025
1 parent 990a339 commit 394709e
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 17 deletions.
26 changes: 13 additions & 13 deletions komorebi/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -244,16 +244,10 @@ fn main() -> Result<()> {
StaticConfig::postload(config, &wm)?;
}

listen_for_commands(wm.clone());

if !opts.await_configuration && !INITIAL_CONFIGURATION_LOADED.load(Ordering::SeqCst) {
INITIAL_CONFIGURATION_LOADED.store(true, Ordering::SeqCst);
};

if let Some(port) = opts.tcp_port {
listen_for_commands_tcp(wm.clone(), port);
}

if static_config.is_none() {
std::thread::spawn(|| load_configuration().expect("could not load configuration"));

Expand All @@ -280,21 +274,27 @@ fn main() -> Result<()> {

wm.lock().retile_all(false)?;

listen_for_events(wm.clone());

if CUSTOM_FFM.load(Ordering::SeqCst) {
listen_for_movements(wm.clone());
}

border_manager::listen_for_notifications(wm.clone());
stackbar_manager::listen_for_notifications(wm.clone());
transparency_manager::listen_for_notifications(wm.clone());
workspace_reconciliator::listen_for_notifications(wm.clone());
monitor_reconciliator::listen_for_notifications(wm.clone())?;
reaper::listen_for_notifications(wm.clone());
reaper::listen_for_notifications(wm.clone(), wm.lock().known_hwnds.clone());
focus_manager::listen_for_notifications(wm.clone());
theme_manager::listen_for_notifications();

listen_for_commands(wm.clone());

if let Some(port) = opts.tcp_port {
listen_for_commands_tcp(wm.clone(), port);
}

listen_for_events(wm.clone());

if CUSTOM_FFM.load(Ordering::SeqCst) {
listen_for_movements(wm.clone());
}

let (ctrlc_sender, ctrlc_receiver) = crossbeam_channel::bounded(1);
ctrlc::set_handler(move || {
ctrlc_sender
Expand Down
11 changes: 7 additions & 4 deletions komorebi/src/reaper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,11 @@ pub fn send_notification(hwnds: HashMap<isize, (usize, usize)>) {
}
}

pub fn listen_for_notifications(wm: Arc<Mutex<WindowManager>>) {
watch_for_orphans(wm.clone());
pub fn listen_for_notifications(
wm: Arc<Mutex<WindowManager>>,
known_hwnds: HashMap<isize, (usize, usize)>,
) {
watch_for_orphans(known_hwnds);

std::thread::spawn(move || loop {
match handle_notifications(wm.clone()) {
Expand Down Expand Up @@ -138,11 +141,11 @@ fn handle_notifications(wm: Arc<Mutex<WindowManager>>) -> color_eyre::Result<()>
Ok(())
}

fn watch_for_orphans(wm: Arc<Mutex<WindowManager>>) {
fn watch_for_orphans(known_hwnds: HashMap<isize, (usize, usize)>) {
// Cache current hwnds
{
let mut cache = HWNDS_CACHE.lock();
*cache = wm.lock().known_hwnds.clone();
*cache = known_hwnds;
}

std::thread::spawn(move || loop {
Expand Down

0 comments on commit 394709e

Please sign in to comment.