From 3f6ff534d3ad3df8bcf93dd02e1dd6dc3da1d7c2 Mon Sep 17 00:00:00 2001 From: Wez Furlong Date: Fri, 28 May 2021 15:08:21 -0700 Subject: [PATCH] windows: fix lingering cmd.exe panes 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. --- Cargo.lock | 3 ++- docs/changelog.md | 1 + mux/Cargo.toml | 5 +++++ mux/src/lib.rs | 1 + mux/src/localpane.rs | 44 ++++++++++++++++++++++++++++++++++++++++++ mux/src/ssh.rs | 5 +++++ pty/Cargo.toml | 2 +- pty/src/lib.rs | 9 +++++++++ pty/src/serial.rs | 5 +++++ pty/src/ssh.rs | 5 +++++ pty/src/win/mod.rs | 5 +++++ wezterm-ssh/Cargo.toml | 2 +- wezterm-ssh/src/pty.rs | 5 +++++ 13 files changed, 89 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 22f82ff897c..5cbe22ae5e3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2514,6 +2514,7 @@ dependencies = [ "url", "wezterm-ssh", "wezterm-term", + "winapi 0.3.9", ] [[package]] @@ -3165,7 +3166,7 @@ dependencies = [ [[package]] name = "portable-pty" -version = "0.4.0" +version = "0.5.0" dependencies = [ "anyhow", "bitflags", diff --git a/docs/changelog.md b/docs/changelog.md index 0d6841e1581..98602a6d15e 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -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 diff --git a/mux/Cargo.toml b/mux/Cargo.toml index e4733e219c3..d7c149b282b 100644 --- a/mux/Cargo.toml +++ b/mux/Cargo.toml @@ -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" diff --git a/mux/src/lib.rs b/mux/src/lib.rs index 7ced652a052..827933d0eb9 100644 --- a/mux/src/lib.rs +++ b/mux/src/lib.rs @@ -187,6 +187,7 @@ fn read_from_pane_pty(pane_id: PaneId, banner: Option, mut reader: Box, 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, diff --git a/mux/src/ssh.rs b/mux/src/ssh.rs index e0ab1e349b6..df88855387e 100644 --- a/mux/src/ssh.rs +++ b/mux/src/ssh.rs @@ -708,6 +708,11 @@ impl portable_pty::Child for WrappedSshChild { fn process_id(&self) -> Option { None } + + #[cfg(windows)] + fn as_raw_handle(&self) -> Option { + None + } } type BoxedReader = Box<(dyn Read + Send + 'static)>; diff --git a/pty/Cargo.toml b/pty/Cargo.toml index ea36e8e16c4..0b024b57ead 100644 --- a/pty/Cargo.toml +++ b/pty/Cargo.toml @@ -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" diff --git a/pty/src/lib.rs b/pty/src/lib.rs index 93cbb25ab2f..e3b9ee6f5fe 100644 --- a/pty/src/lib.rs +++ b/pty/src/lib.rs @@ -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; + /// Returns the process handle of the child process, if applicable. + /// Only available on Windows. + #[cfg(windows)] + fn as_raw_handle(&self) -> Option; } /// Represents the slave side of a pty. @@ -232,6 +236,11 @@ impl Child for std::process::Child { fn process_id(&self) -> Option { Some(self.id()) } + + #[cfg(windows)] + fn as_raw_handle(&self) -> Option { + Some(std::os::windows::io::AsRawHandle::as_raw_handle(self)) + } } pub fn native_pty_system() -> Box { diff --git a/pty/src/serial.rs b/pty/src/serial.rs index 5f1bfc59f55..3a90c27d5bd 100644 --- a/pty/src/serial.rs +++ b/pty/src/serial.rs @@ -142,6 +142,11 @@ impl Child for SerialChild { fn process_id(&self) -> Option { None } + + #[cfg(windows)] + fn as_raw_handle(&self) -> Option { + None + } } struct Master { diff --git a/pty/src/ssh.rs b/pty/src/ssh.rs index 454c26c5a7d..f9dc8c51ed7 100644 --- a/pty/src/ssh.rs +++ b/pty/src/ssh.rs @@ -260,6 +260,11 @@ impl Child for SshChild { fn process_id(&self) -> Option { None } + + #[cfg(windows)] + fn as_raw_handle(&self) -> Option { + None + } } struct SshReader { diff --git a/pty/src/win/mod.rs b/pty/src/win/mod.rs index 7c85f6a28f6..815acfc2825 100644 --- a/pty/src/win/mod.rs +++ b/pty/src/win/mod.rs @@ -86,6 +86,11 @@ impl Child for WinChild { Some(res) } } + + fn as_raw_handle(&self) -> Option { + let proc = self.proc.lock().unwrap(); + Some(proc.as_raw_handle()) + } } impl std::future::Future for WinChild { diff --git a/wezterm-ssh/Cargo.toml b/wezterm-ssh/Cargo.toml index 77e914c4bfe..7b1347deb28 100644 --- a/wezterm-ssh/Cargo.toml +++ b/wezterm-ssh/Cargo.toml @@ -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"]} diff --git a/wezterm-ssh/src/pty.rs b/wezterm-ssh/src/pty.rs index b9fd64ec552..515d2a68873 100644 --- a/wezterm-ssh/src/pty.rs +++ b/wezterm-ssh/src/pty.rs @@ -133,6 +133,11 @@ impl portable_pty::Child for SshChildProcess { fn process_id(&self) -> Option { None } + + #[cfg(windows)] + fn as_raw_handle(&self) -> Option { + None + } } impl crate::session::SessionInner {