Skip to content

Commit

Permalink
fix(wm): prevent hidden_hwnds deadlock
Browse files Browse the repository at this point in the history
I used a parking_lot to detect what I suspected to be the deadlock
resulting in issue #13.

I was pleasantly surprised by the alternative to std::sync::Mutex
provided by parking_lot, especially not having to unlock it to use it,
and of course the excellent and intuitive (even if experimental)
deadlock detector.

I have decided to use parking_lot::Mutex as an almost-drop-in
replacement for std::sync::Mutex, as I expect that this isn't the last
time komorebi will have a deadlocking issue, and I have put the deadlock
detection code which runs in a separate thread behind a
"deadlock_detection" feature.

The actual deadlock itself was solved by scoping the first lock in the
handler for WindowManagerEvent::Hide and then executing any required
operations (some of which, like window.maximize(), may require another
lock on HIDDEN_HWNDS) in a separate scope once the previous lock has
been dropped.

In the future I should look at integrating globals like HIDDEN_HWNDS
into WindowManager in a way that won't lead to double-mutable-borrow
issues.

fix #13
  • Loading branch information
LGUG2Z committed Aug 19, 2021
1 parent 98f731b commit 209cd82
Show file tree
Hide file tree
Showing 10 changed files with 201 additions and 48 deletions.
84 changes: 82 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

30 changes: 28 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ This means that:

## Getting Started

### GitHub Releases

Prebuilt binaries are available on the [releases page](https://github.com/LGUG2Z/komorebi/releases) in a `zip` archive.
Once downloaded, you will need to move the `komorebi.exe` and `komorebic.exe` binaries to a directory in your `Path` (
you can see these directories by running `$Env:Path.split(";")` at a PowerShell prompt).
Expand All @@ -68,6 +70,8 @@ using [`setx`](https://docs.microsoft.com/en-us/windows-server/administration/wi
Variables pop up in System Properties Advanced (which can be launched with `SystemPropertiesAdvanced.exe` at a
PowerShell prompt), and then move the binaries to that directory.

### Scoop

If you use the [Scoop](https://scoop.sh/) command line installer, you can run the following commands to install the
binaries from the latest GitHub Release:

Expand All @@ -79,6 +83,8 @@ scoop install komorebi
If you install _komorebi_ using Scoop, the binaries will automatically be added to your `Path` and a command will be
shown for you to run in order to get started using the sample configuration file.

### Building from Source

If you prefer to compile _komorebi_ from source, you will need
a [working Rust development environment on Windows 10](https://rustup.rs/). The `x86_64-pc-windows-msvc` toolchain is
required, so make sure you have also installed
Expand All @@ -91,6 +97,8 @@ cargo install --path komorebi --locked
cargo install --path komorebic --locked
```

### Running

Once you have either the prebuilt binaries in your `Path`, or have compiled the binaries from source (these will already
be in your `Path` if you installed Rust with [rustup](https://rustup.rs), which you absolutely should), you can
run `komorebic start` at a Powershell prompt, and you will see the following output:
Expand All @@ -102,6 +110,8 @@ Start-Process komorebi -WindowStyle hidden
This means that `komorebi` is now running in the background, tiling all your windows, and listening for commands sent to
it by `komorebic`. You can similarly stop the process by running `komorebic stop`.

### Configuring

Once `komorebi` is running, you can execute the `komorebi.sample.ahk` script to set up the default keybindings via AHK
(the file includes comments to help you start building your own configuration).

Expand All @@ -113,6 +123,8 @@ the `AutoHotKey64.exe` executable for AutoHotKey v2 is in your `Path`. If both `
exist in your home directory, only `komorebi.ahk` will be loaded. An example of an AutoHotKey v2 configuration file
for _komorebi_ can be found [here](https://gist.github.com/crosstyan/dafacc0778dabf693ce9236c57b201cd).

### Common First-Time Troubleshooting

If you are experiencing behaviour where
[closing a window leaves a blank tile, but minimizing the same window does not](https://github.com/LGUG2Z/komorebi/issues/6)
, you have probably enabled a 'close/minimize to tray' option for that application. You can tell _komorebi_ to handle
Expand All @@ -123,7 +135,7 @@ komorebic.exe identify-tray-application exe Discord.exe
komorebic.exe identify-tray-application exe Telegram.exe
```

## Configuration
## Configuration with `komorebic`

As previously mentioned, this project does not handle anything related to keybindings and shortcuts directly. I
personally use AutoHotKey to manage my window management shortcuts, and have provided a
Expand Down Expand Up @@ -151,6 +163,7 @@ focus-workspace Focus the specified workspace on the focused monito
new-workspace Create and append a new workspace on the focused monitor
adjust-container-padding Adjust container padding on the focused workspace
adjust-workspace-padding Adjust workspace padding on the focused workspace
change-layout Set the layout on the focused workspace
flip-layout Flip the layout on the focused workspace (BSP only)
promote Promote the focused window to the top of the tree
retile Force the retiling of all managed windows
Expand All @@ -164,7 +177,7 @@ toggle-pause Toggle the window manager on and off across all mon
toggle-tiling Toggle window tiling on the focused workspace
toggle-float Toggle floating mode for the focused window
toggle-monocle Toggle monocle mode for the focused container
toggle-maximize Toggle native window fullscreen for the focused window
toggle-maximize Toggle native maximization for the focused window
restore-windows Restore all hidden windows (debugging command)
reload-configuration Reload ~/komorebi.ahk (if it exists)
watch-configuration Toggle the automatic reloading of ~/komorebi.ahk (if it exists)
Expand Down Expand Up @@ -242,9 +255,22 @@ ensures that all hidden windows are restored before termination.
If however, you ever end up with windows that are hidden and cannot be restored, a list of window handles known
to `komorebi` are stored and continuously updated in `~/komorebi.hwnd.json`.

### Restoring Windows

Running `komorebic restore-windows` will read the list of window handles and forcibly restore them, regardless of
whether the main `komorebi` process is running.

### Panics and Deadlocks

If `komorebi` ever stops responding, it is most likely either due to either a panic or a deadlock. In the case of a
panic, this will be reported in the log. In the case of a deadlock, there will not be any errors in the log, but the
process and the log will appear frozen.

If you believe you have encountered a deadlock, you can compile `komorebi` with `--feature deadlock_detection` and try
reproducing the deadlock again. This will check for deadlocks every 5 seconds in the background, and if a deadlock is
found, information about it will appear in the log which can be shared when opening an issu which can be shared when
opening an issue.

## Window Manager State and Integrations

The current state of the window manager can be queried using the `komorebic state` command, which returns a JSON
Expand Down
6 changes: 5 additions & 1 deletion komorebi/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ getset = "0.1"
hotwatch = "0.4"
lazy_static = "1"
nanoid = "0.4"
parking_lot = { version = "0.11", features = ["deadlock_detection"] }
paste = "1"
serde = { version = "1", features = ["derive"] }
serde_json = "1"
Expand All @@ -29,4 +30,7 @@ tracing = "0.1"
tracing-appender = "0.1"
tracing-subscriber = "0.2"
uds_windows = "1"
which = "4"
which = "4"

[features]
deadlock_detection = []
40 changes: 36 additions & 4 deletions komorebi/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,19 @@

use std::process::Command;
use std::sync::Arc;
use std::sync::Mutex;
#[cfg(feature = "deadlock_detection")]
use std::thread;
#[cfg(feature = "deadlock_detection")]
use std::time::Duration;

use color_eyre::eyre::ContextCompat;
use color_eyre::Result;
use crossbeam_channel::Receiver;
use crossbeam_channel::Sender;
use lazy_static::lazy_static;
#[cfg(feature = "deadlock_detection")]
use parking_lot::deadlock;
use parking_lot::Mutex;
use sysinfo::SystemExt;
use tracing_appender::non_blocking::WorkerGuard;
use tracing_subscriber::layer::SubscriberExt;
Expand Down Expand Up @@ -54,7 +60,7 @@ lazy_static! {
"chrome.exe".to_string(),
"idea64.exe".to_string(),
"ApplicationFrameHost.exe".to_string(),
"steam.exe".to_string()
"steam.exe".to_string(),
]));
static ref OBJECT_NAME_CHANGE_ON_LAUNCH: Arc<Mutex<Vec<String>>> = Arc::new(Mutex::new(vec![
"firefox.exe".to_string(),
Expand Down Expand Up @@ -161,6 +167,29 @@ pub fn load_configuration() -> Result<()> {
Ok(())
}

#[cfg(feature = "deadlock_detection")]
#[tracing::instrument]
fn detect_deadlocks() {
// Create a background thread which checks for deadlocks every 10s
thread::spawn(move || loop {
tracing::info!("running deadlock detector");
thread::sleep(Duration::from_secs(5));
let deadlocks = deadlock::check_deadlock();
if deadlocks.is_empty() {
continue;
}

tracing::error!("{} deadlocks detected", deadlocks.len());
for (i, threads) in deadlocks.iter().enumerate() {
tracing::error!("deadlock #{}", i);
for t in threads {
tracing::error!("thread id: {:#?}", t.thread_id());
tracing::error!("{:#?}", t.backtrace());
}
}
});
}

#[tracing::instrument]
fn main() -> Result<()> {
match std::env::args().count() {
Expand All @@ -176,6 +205,9 @@ fn main() -> Result<()> {
// File logging worker guard has to have an assignment in the main fn to work
let (_guard, _color_guard) = setup()?;

#[cfg(feature = "deadlock_detection")]
detect_deadlocks();

let process_id = WindowsApi::current_process_id();
WindowsApi::allow_set_foreground_window(process_id)?;

Expand All @@ -189,7 +221,7 @@ fn main() -> Result<()> {
incoming,
)))?));

wm.lock().unwrap().init()?;
wm.lock().init()?;
listen_for_commands(wm.clone());
listen_for_events(wm.clone());

Expand All @@ -210,7 +242,7 @@ fn main() -> Result<()> {
"received ctrl-c, restoring all hidden windows and terminating process"
);

wm.lock().unwrap().restore_all_windows();
wm.lock().restore_all_windows();
std::process::exit(130);
}
_ => Ok(()),
Expand Down
Loading

0 comments on commit 209cd82

Please sign in to comment.