Skip to content

Commit

Permalink
kill and reap ProxyCommand
Browse files Browse the repository at this point in the history
The wrapper struct ensures ensures the child process spawned for
the ProxyCommand is cleaned up under all the various error scenarios
(such as auth failures etc), as well as in the normal successful use
case. This includes killing it if necessary, and then waiting for it.
  • Loading branch information
daaku authored and wez committed Jul 15, 2024
1 parent 57be095 commit 463df9e
Showing 1 changed file with 27 additions and 7 deletions.
34 changes: 27 additions & 7 deletions wezterm-ssh/src/sessioninner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ impl SessionInner {
sess.set_option(libssh_rs::SshOption::HostKeys(host_key.to_string()))?;
}

let sock =
let (sock, _child) =
self.connect_to_host(&hostname, port, verbose, self.config.get("proxycommand"))?;
let raw = {
#[cfg(unix)]
Expand Down Expand Up @@ -288,7 +288,7 @@ impl SessionInner {
))))
.context("notifying user of banner")?;

let sock =
let (sock, _child) =
self.connect_to_host(&hostname, port, verbose, self.config.get("proxycommand"))?;

let mut sess = ssh2::Session::new()?;
Expand Down Expand Up @@ -332,7 +332,7 @@ impl SessionInner {
port: u16,
verbose: bool,
proxy_command: Option<&String>,
) -> anyhow::Result<Socket> {
) -> anyhow::Result<(Socket, Option<KillOnDropChild>)> {
match proxy_command.map(|s| s.as_str()) {
Some("none") | None => {}
Some(proxy_command) => {
Expand All @@ -351,19 +351,25 @@ impl SessionInner {
cmd.stdin(b.as_stdio()?);
cmd.stdout(b.as_stdio()?);
cmd.stderr(std::process::Stdio::inherit());
let _child = cmd
let child = cmd
.spawn()
.with_context(|| format!("spawning ProxyCommand {}", proxy_command))?;

#[cfg(unix)]
unsafe {
use std::os::unix::io::{FromRawFd, IntoRawFd};
return Ok(Socket::from_raw_fd(a.into_raw_fd()));
return Ok((
Socket::from_raw_fd(a.into_raw_fd()),
Some(KillOnDropChild(child)),
));
}
#[cfg(windows)]
unsafe {
use std::os::windows::io::{FromRawSocket, IntoRawSocket};
return Ok(Socket::from_raw_socket(a.into_raw_socket()));
return Ok((
Socket::from_raw_socket(a.into_raw_socket()),
Some(KillOnDropChild(child)),
));
}
}
}
Expand Down Expand Up @@ -392,7 +398,7 @@ impl SessionInner {

sock.connect(&addr.into())
.with_context(|| format!("Connecting to {hostname}:{port} ({addr:?})"))?;
Ok(sock)
Ok((sock, None))
}

/// Used to restrict to_socket_addrs results to the address
Expand Down Expand Up @@ -1086,3 +1092,17 @@ where
}
Ok(true)
}

/// A little helper to ensure the Child process is killed on Drop.
struct KillOnDropChild(std::process::Child);

impl Drop for KillOnDropChild {
fn drop(&mut self) {
if let Err(err) = self.0.kill() {
log::error!("Error killing ProxyCommand: {}", err);
}
if let Err(err) = self.0.wait() {
log::error!("Error waiting for ProxyCommand to finish: {}", err);
}
}
}

0 comments on commit 463df9e

Please sign in to comment.