Skip to content

Commit

Permalink
windows: fix lingering cmd.exe panes
Browse files Browse the repository at this point in the history
Since removing the regular periodic background tasks, we're now
prone to not noticing child processes exiting.

This commit explicictly schedules a thread to do that on Windows
so that we can close a tab as soon as it exits.
  • Loading branch information
wez committed May 28, 2021
1 parent 68619fc commit 3f6ff53
Show file tree
Hide file tree
Showing 13 changed files with 89 additions and 3 deletions.
3 changes: 2 additions & 1 deletion Cargo.lock

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

1 change: 1 addition & 0 deletions docs/changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ As features stabilize some brief notes about them will accumulate here.
* Changed: the homebrew tap is now a Cask that installs to the /Applications directory on macOS. Thanks to [@laggardkernel](https://github.com/laggardkernel)!
* New: bold and/or italics are now synthesized for fonts when the matching font is not actually italic or doesn't match the requested weight. [#815](https://github.com/wez/wezterm/issues/815)
* Updated: conpty.dll to v1.9.1445.0; fixes color bar artifacts when resizing window and allows win32 console applications to use mouse events
* Fixed: Windows: pane could linger after the process has died, closing only when a new pane/tab event occurs

### 20210502-154244-3f7122cb

Expand Down
5 changes: 5 additions & 0 deletions mux/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -39,5 +39,10 @@ wezterm-term = { path = "../term", features=["use_serde"] }
[target.'cfg(any(windows, target_os="linux", target_os="macos"))'.dependencies]
sysinfo = "0.16"

[target."cfg(windows)".dependencies]
winapi = { version = "0.3", features = [
"handleapi",
]}

[dev-dependencies]
k9 = "0.11"
1 change: 1 addition & 0 deletions mux/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@ fn read_from_pane_pty(pane_id: PaneId, banner: Option<String>, mut reader: Box<d
// this pane right now, so don't!
promise::spawn::spawn_into_main_thread(async move {
let mux = Mux::get().unwrap();
log::trace!("checking for dead windows after EOF on pane {}", pane_id);
mux.prune_dead_windows();
})
.detach();
Expand Down
44 changes: 44 additions & 0 deletions mux/src/localpane.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use anyhow::Error;
use async_trait::async_trait;
use config::keyassignment::ScrollbackEraseMode;
use config::{configuration, ExitBehavior};
use filedescriptor::OwnedHandle;
use portable_pty::{Child, MasterPty, PtySize};
use rangeset::RangeSet;
use std::cell::{RefCell, RefMut};
Expand Down Expand Up @@ -536,6 +537,49 @@ impl LocalPane {
pty: Box<dyn MasterPty>,
domain_id: DomainId,
) -> Self {
// This is a little gross; on Windows, our pipe reader will continue
// to be blocked in read even after the child process has died.
// We need to wake up and notice that the child terminated in order
// for our state to wind down.
// This block schedules a background thread to wait for the child
// to terminate, and then nudge the muxer to check for dead processes.
// Without this, typing `exit` in `cmd.exe` would keep the pane around
// until something else triggered the mux to prune dead processes.
#[cfg(windows)]
{
use std::os::windows::io::{AsRawHandle, RawHandle};
use winapi::um::synchapi::WaitForSingleObject;
use winapi::um::winbase::INFINITE;

struct RawDup(RawHandle);
impl AsRawHandle for RawDup {
fn as_raw_handle(&self) -> RawHandle {
self.0
}
}

if let Some(Ok(handle)) = process
.as_raw_handle()
.as_ref()
.map(|h| OwnedHandle::dup(&RawDup(*h)))
{
std::thread::spawn(move || {
unsafe {
WaitForSingleObject(handle.as_raw_handle(), INFINITE);
};
promise::spawn::spawn_into_main_thread(async move {
let mux = Mux::get().unwrap();
log::trace!(
"checking for dead windows after child died in pane {}",
pane_id
);
mux.prune_dead_windows();
})
.detach();
});
}
}

terminal.set_device_control_handler(Box::new(LocalPaneDCSHandler {
pane_id,
tmux_domain: None,
Expand Down
5 changes: 5 additions & 0 deletions mux/src/ssh.rs
Original file line number Diff line number Diff line change
Expand Up @@ -708,6 +708,11 @@ impl portable_pty::Child for WrappedSshChild {
fn process_id(&self) -> Option<u32> {
None
}

#[cfg(windows)]
fn as_raw_handle(&self) -> Option<std::os::windows::io::RawHandle> {
None
}
}

type BoxedReader = Box<(dyn Read + Send + 'static)>;
Expand Down
2 changes: 1 addition & 1 deletion pty/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "portable-pty"
version = "0.4.0"
version = "0.5.0"
authors = ["Wez Furlong"]
edition = "2018"
repository = "https://github.com/wez/wezterm"
Expand Down
9 changes: 9 additions & 0 deletions pty/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,10 @@ pub trait Child: std::fmt::Debug {
/// Returns the process identifier of the child process,
/// if applicable
fn process_id(&self) -> Option<u32>;
/// Returns the process handle of the child process, if applicable.
/// Only available on Windows.
#[cfg(windows)]
fn as_raw_handle(&self) -> Option<std::os::windows::io::RawHandle>;
}

/// Represents the slave side of a pty.
Expand Down Expand Up @@ -232,6 +236,11 @@ impl Child for std::process::Child {
fn process_id(&self) -> Option<u32> {
Some(self.id())
}

#[cfg(windows)]
fn as_raw_handle(&self) -> Option<std::os::windows::io::RawHandle> {
Some(std::os::windows::io::AsRawHandle::as_raw_handle(self))
}
}

pub fn native_pty_system() -> Box<dyn PtySystem> {
Expand Down
5 changes: 5 additions & 0 deletions pty/src/serial.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,11 @@ impl Child for SerialChild {
fn process_id(&self) -> Option<u32> {
None
}

#[cfg(windows)]
fn as_raw_handle(&self) -> Option<std::os::windows::io::RawHandle> {
None
}
}

struct Master {
Expand Down
5 changes: 5 additions & 0 deletions pty/src/ssh.rs
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,11 @@ impl Child for SshChild {
fn process_id(&self) -> Option<u32> {
None
}

#[cfg(windows)]
fn as_raw_handle(&self) -> Option<std::os::windows::io::RawHandle> {
None
}
}

struct SshReader {
Expand Down
5 changes: 5 additions & 0 deletions pty/src/win/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,11 @@ impl Child for WinChild {
Some(res)
}
}

fn as_raw_handle(&self) -> Option<std::os::windows::io::RawHandle> {
let proc = self.proc.lock().unwrap();
Some(proc.as_raw_handle())
}
}

impl std::future::Future for WinChild {
Expand Down
2 changes: 1 addition & 1 deletion wezterm-ssh/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ base64 = "0.13"
dirs-next = "2.0"
filedescriptor = { version="0.8", path = "../filedescriptor" }
log = "0.4"
portable-pty = { version="0.4", path = "../pty" }
portable-pty = { version="0.5", path = "../pty" }
regex = "1"
smol = "1.2"
ssh2 = {version="0.9", features=["openssl-on-win32"]}
Expand Down
5 changes: 5 additions & 0 deletions wezterm-ssh/src/pty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,11 @@ impl portable_pty::Child for SshChildProcess {
fn process_id(&self) -> Option<u32> {
None
}

#[cfg(windows)]
fn as_raw_handle(&self) -> Option<std::os::windows::io::RawHandle> {
None
}
}

impl crate::session::SessionInner {
Expand Down

0 comments on commit 3f6ff53

Please sign in to comment.