fix(wm): remove blocking channel send calls

This commit is the result of a long investigation with @berknam on
Discord which uncovered that when the channels used by the *_manager
modules are full, the window manager can enter a completely locked state
which will require a hard restart of komorebi.exe.

In order to avoid entering this locked state, *_manager modules now no
longer publicly expose event_tx for sending notifications.

Instead, a new public fn send_notification is exposed which will use
try_send to attempt to send notifications in a non-blocking manner and
log warnings if the channel is full and the notification is dropped.
This commit is contained in:
LGUG2Z
2024-06-19 10:12:53 -07:00
parent 31864b1570
commit 8bf4ab9f15
10 changed files with 277 additions and 184 deletions
+8 -2
View File
@@ -62,14 +62,20 @@ pub fn channel() -> &'static (Sender<Notification>, Receiver<Notification>) {
CHANNEL.get_or_init(|| crossbeam_channel::bounded(20))
}
pub fn event_tx() -> Sender<Notification> {
fn event_tx() -> Sender<Notification> {
channel().0.clone()
}
pub fn event_rx() -> Receiver<Notification> {
fn event_rx() -> Receiver<Notification> {
channel().1.clone()
}
pub fn send_notification() {
if event_tx().try_send(Notification).is_err() {
tracing::warn!("channel is full; dropping notification")
}
}
pub fn destroy_all_borders() -> color_eyre::Result<()> {
let mut borders = BORDER_STATE.lock();
tracing::info!(
+19 -13
View File
@@ -103,7 +103,7 @@ impl Hidden {
tracing::debug!(
"WM_POWERBROADCAST event received - resume from suspend"
);
let _ = monitor_reconciliator::event_tx().send(
monitor_reconciliator::send_notification(
monitor_reconciliator::Notification::ResumingFromSuspendedState,
);
LRESULT(0)
@@ -113,8 +113,9 @@ impl Hidden {
tracing::debug!(
"WM_POWERBROADCAST event received - entering suspended state"
);
let _ = monitor_reconciliator::event_tx()
.send(monitor_reconciliator::Notification::EnteringSuspendedState);
monitor_reconciliator::send_notification(
monitor_reconciliator::Notification::EnteringSuspendedState,
);
LRESULT(0)
}
_ => LRESULT(0),
@@ -125,14 +126,16 @@ impl Hidden {
WTS_SESSION_LOCK => {
tracing::debug!("WM_WTSSESSION_CHANGE event received with WTS_SESSION_LOCK - screen locked");
let _ = monitor_reconciliator::event_tx()
.send(monitor_reconciliator::Notification::SessionLocked);
monitor_reconciliator::send_notification(
monitor_reconciliator::Notification::SessionLocked,
);
}
WTS_SESSION_UNLOCK => {
tracing::debug!("WM_WTSSESSION_CHANGE event received with WTS_SESSION_UNLOCK - screen unlocked");
let _ = monitor_reconciliator::event_tx()
.send(monitor_reconciliator::Notification::SessionUnlocked);
monitor_reconciliator::send_notification(
monitor_reconciliator::Notification::SessionUnlocked,
);
}
_ => {}
}
@@ -151,8 +154,9 @@ impl Hidden {
"WM_DISPLAYCHANGE event received with wparam: {}- work area or display resolution changed", wparam.0
);
let _ = monitor_reconciliator::event_tx()
.send(monitor_reconciliator::Notification::ResolutionScalingChanged);
monitor_reconciliator::send_notification(
monitor_reconciliator::Notification::ResolutionScalingChanged,
);
LRESULT(0)
}
// Unfortunately this is the event sent with ButteryTaskbar which I use a lot
@@ -164,8 +168,9 @@ impl Hidden {
"WM_SETTINGCHANGE event received with SPI_SETWORKAREA - work area changed (probably butterytaskbar or something similar)"
);
let _ = monitor_reconciliator::event_tx()
.send(monitor_reconciliator::Notification::WorkAreaChanged);
monitor_reconciliator::send_notification(
monitor_reconciliator::Notification::WorkAreaChanged,
);
}
LRESULT(0)
}
@@ -177,8 +182,9 @@ impl Hidden {
tracing::debug!(
"WM_DEVICECHANGE event received with DBT_DEVNODES_CHANGED - display added or removed"
);
let _ = monitor_reconciliator::event_tx()
.send(monitor_reconciliator::Notification::DisplayConnectionChange);
monitor_reconciliator::send_notification(
monitor_reconciliator::Notification::DisplayConnectionChange,
);
}
LRESULT(0)
+11 -5
View File
@@ -40,14 +40,20 @@ pub fn channel() -> &'static (Sender<Notification>, Receiver<Notification>) {
CHANNEL.get_or_init(|| crossbeam_channel::bounded(1))
}
pub fn event_tx() -> Sender<Notification> {
fn event_tx() -> Sender<Notification> {
channel().0.clone()
}
pub fn event_rx() -> Receiver<Notification> {
fn event_rx() -> Receiver<Notification> {
channel().1.clone()
}
pub fn send_notification(notification: Notification) {
if event_tx().try_send(notification).is_err() {
tracing::warn!("channel is full; dropping notification")
}
}
pub fn insert_in_monitor_cache(device_id: &str, config: MonitorConfig) {
let mut monitor_cache = MONITOR_CACHE
.get_or_init(|| Mutex::new(HashMap::new()))
@@ -160,7 +166,7 @@ pub fn handle_notifications(wm: Arc<Mutex<WindowManager>>) -> color_eyre::Result
if should_update {
tracing::info!("updated work area for {}", monitor.device_id());
monitor.update_focused_workspace(offset)?;
border_manager::event_tx().send(border_manager::Notification)?;
border_manager::send_notification();
} else {
tracing::debug!(
"work areas match, reconciliation not required for {}",
@@ -207,7 +213,7 @@ pub fn handle_notifications(wm: Arc<Mutex<WindowManager>>) -> color_eyre::Result
);
monitor.update_focused_workspace(offset)?;
border_manager::event_tx().send(border_manager::Notification)?;
border_manager::send_notification();
} else {
tracing::debug!(
"resolutions match, reconciliation not required for {}",
@@ -394,7 +400,7 @@ pub fn handle_notifications(wm: Arc<Mutex<WindowManager>>) -> color_eyre::Result
// Second retile to fix DPI/resolution related jank
wm.retile_all(true)?;
// Border updates to fix DPI/resolution related jank
border_manager::event_tx().send(border_manager::Notification)?;
border_manager::send_notification();
}
}
}
+3 -3
View File
@@ -1360,9 +1360,9 @@ impl WindowManager {
};
notify_subscribers(&serde_json::to_string(&notification)?)?;
border_manager::event_tx().send(border_manager::Notification)?;
transparency_manager::event_tx().send(transparency_manager::Notification)?;
stackbar_manager::event_tx().send(stackbar_manager::Notification)?;
border_manager::send_notification();
transparency_manager::send_notification();
stackbar_manager::send_notification();
tracing::info!("processed");
Ok(())
+6 -11
View File
@@ -45,7 +45,8 @@ pub fn listen_for_events(wm: Arc<Mutex<WindowManager>>) {
tracing::info!("listening");
loop {
if let Ok(event) = receiver.recv() {
match wm.lock().process_event(event) {
let mut guard = wm.lock();
match guard.process_event(event) {
Ok(()) => {}
Err(error) => {
if cfg!(debug_assertions) {
@@ -298,13 +299,7 @@ impl WindowManager {
}
}
workspace_reconciliator::event_tx().send(
workspace_reconciliator::Notification {
monitor_idx: i,
workspace_idx: j,
},
)?;
workspace_reconciliator::send_notification(i, j);
needs_reconciliation = true;
}
}
@@ -635,9 +630,9 @@ impl WindowManager {
};
notify_subscribers(&serde_json::to_string(&notification)?)?;
border_manager::event_tx().send(border_manager::Notification)?;
transparency_manager::event_tx().send(transparency_manager::Notification)?;
stackbar_manager::event_tx().send(stackbar_manager::Notification)?;
border_manager::send_notification();
transparency_manager::send_notification();
stackbar_manager::send_notification();
// Too many spammy OBJECT_NAMECHANGE events from JetBrains IDEs
if !matches!(
+1 -1
View File
@@ -51,7 +51,7 @@ pub fn find_orphans(wm: Arc<Mutex<WindowManager>>) -> color_eyre::Result<()> {
let reaped_orphans = workspace.reap_orphans()?;
if reaped_orphans.0 > 0 || reaped_orphans.1 > 0 {
workspace.update(&work_area, offset, window_based_work_area_offset)?;
border_manager::event_tx().send(border_manager::Notification)?;
border_manager::send_notification();
tracing::info!(
"reaped {} orphan window(s) and {} orphaned container(s) on monitor: {}, workspace: {}",
reaped_orphans.0,
+8 -2
View File
@@ -43,14 +43,20 @@ pub fn channel() -> &'static (Sender<Notification>, Receiver<Notification>) {
CHANNEL.get_or_init(|| crossbeam_channel::bounded(20))
}
pub fn event_tx() -> Sender<Notification> {
fn event_tx() -> Sender<Notification> {
channel().0.clone()
}
pub fn event_rx() -> Receiver<Notification> {
fn event_rx() -> Receiver<Notification> {
channel().1.clone()
}
pub fn send_notification() {
if event_tx().try_send(Notification).is_err() {
tracing::warn!("channel is full; dropping notification")
}
}
pub fn should_have_stackbar(window_count: usize) -> bool {
match STACKBAR_MODE.load() {
StackbarMode::Always => true,
+8 -2
View File
@@ -32,14 +32,20 @@ pub fn channel() -> &'static (Sender<Notification>, Receiver<Notification>) {
CHANNEL.get_or_init(|| crossbeam_channel::bounded(20))
}
pub fn event_tx() -> Sender<Notification> {
fn event_tx() -> Sender<Notification> {
channel().0.clone()
}
pub fn event_rx() -> Receiver<Notification> {
fn event_rx() -> Receiver<Notification> {
channel().1.clone()
}
pub fn send_notification() {
if event_tx().try_send(Notification).is_err() {
tracing::warn!("channel is full; dropping notification")
}
}
pub fn listen_for_notifications(wm: Arc<Mutex<WindowManager>>) {
std::thread::spawn(move || loop {
match handle_notifications(wm.clone()) {
+15 -3
View File
@@ -30,14 +30,26 @@ pub fn channel() -> &'static (Sender<Notification>, Receiver<Notification>) {
CHANNEL.get_or_init(|| crossbeam_channel::bounded(1))
}
pub fn event_tx() -> Sender<Notification> {
fn event_tx() -> Sender<Notification> {
channel().0.clone()
}
pub fn event_rx() -> Receiver<Notification> {
fn event_rx() -> Receiver<Notification> {
channel().1.clone()
}
pub fn send_notification(monitor_idx: usize, workspace_idx: usize) {
if event_tx()
.try_send(Notification {
monitor_idx,
workspace_idx,
})
.is_err()
{
tracing::warn!("channel is full; dropping notification")
}
}
pub fn listen_for_notifications(wm: Arc<Mutex<WindowManager>>) {
std::thread::spawn(move || loop {
match handle_notifications(wm.clone()) {
@@ -106,7 +118,7 @@ pub fn handle_notifications(wm: Arc<Mutex<WindowManager>>) -> color_eyre::Result
// Unblock the border manager
ALT_TAB_HWND.store(None);
// Send a notification to the border manager to update the borders
border_manager::event_tx().send(border_manager::Notification)?;
border_manager::send_notification();
}
}
}