From 80d486ef03167b19bc24d69cb3150faf5ca019af Mon Sep 17 00:00:00 2001 From: Ives van Hoorne Date: Sun, 31 Jan 2021 19:09:18 +0100 Subject: [PATCH] Add Sync marker to child process created through pty spawn (#437) * Add Sync marker to child process created through pty spawn * Add 'Sync' to Windows build of pty * Wrap proc in WinChild in a Mutex * Make sure the Mutex is not locked for long by cloning --- pty/src/lib.rs | 2 +- pty/src/serial.rs | 2 +- pty/src/ssh.rs | 4 ++-- pty/src/unix.rs | 5 ++++- pty/src/win/conpty.rs | 2 +- pty/src/win/mod.rs | 18 ++++++++++++------ pty/src/win/psuedocon.rs | 5 ++++- 7 files changed, 25 insertions(+), 13 deletions(-) diff --git a/pty/src/lib.rs b/pty/src/lib.rs index e0d05f848..cf33835b1 100644 --- a/pty/src/lib.rs +++ b/pty/src/lib.rs @@ -129,7 +129,7 @@ pub trait Child: std::fmt::Debug { /// Can be used to spawn processes into the pty. pub trait SlavePty { /// Spawns the command specified by the provided CommandBuilder - fn spawn_command(&self, cmd: CommandBuilder) -> Result, Error>; + fn spawn_command(&self, cmd: CommandBuilder) -> Result, Error>; } /// Represents the exit status of a child process. diff --git a/pty/src/serial.rs b/pty/src/serial.rs index 43a87194b..057ca97fd 100644 --- a/pty/src/serial.rs +++ b/pty/src/serial.rs @@ -99,7 +99,7 @@ struct Slave { } impl SlavePty for Slave { - fn spawn_command(&self, cmd: CommandBuilder) -> anyhow::Result> { + fn spawn_command(&self, cmd: CommandBuilder) -> anyhow::Result> { ensure!( cmd.is_default_prog(), "can only use default prog commands with serial tty implementations" diff --git a/pty/src/ssh.rs b/pty/src/ssh.rs index 2c317feb5..af59ebf05 100644 --- a/pty/src/ssh.rs +++ b/pty/src/ssh.rs @@ -195,7 +195,7 @@ struct SshSlave { } impl SlavePty for SshSlave { - fn spawn_command(&self, cmd: CommandBuilder) -> anyhow::Result> { + fn spawn_command(&self, cmd: CommandBuilder) -> anyhow::Result> { self.pty.with_channel(|channel| { for (key, val) in cmd.iter_env_as_str() { if let Err(err) = channel.setenv(key, val) { @@ -213,7 +213,7 @@ impl SlavePty for SshSlave { channel.exec(&command)?; } - let child: Box = Box::new(SshChild { + let child: Box = Box::new(SshChild { pty: self.pty.clone(), }); diff --git a/pty/src/unix.rs b/pty/src/unix.rs index d683e6fef..d72ca5af9 100644 --- a/pty/src/unix.rs +++ b/pty/src/unix.rs @@ -292,7 +292,10 @@ fn cloexec(fd: RawFd) -> Result<(), Error> { } impl SlavePty for UnixSlavePty { - fn spawn_command(&self, builder: CommandBuilder) -> Result, Error> { + fn spawn_command( + &self, + builder: CommandBuilder, + ) -> Result, Error> { Ok(Box::new(self.fd.spawn_command(builder)?)) } } diff --git a/pty/src/win/conpty.rs b/pty/src/win/conpty.rs index 6da7db5d6..091029d53 100644 --- a/pty/src/win/conpty.rs +++ b/pty/src/win/conpty.rs @@ -112,7 +112,7 @@ impl io::Write for ConPtyMasterPty { } impl SlavePty for ConPtySlavePty { - fn spawn_command(&self, cmd: CommandBuilder) -> anyhow::Result> { + fn spawn_command(&self, cmd: CommandBuilder) -> anyhow::Result> { let inner = self.inner.lock().unwrap(); let child = inner.con.spawn_command(cmd)?; Ok(Box::new(child)) diff --git a/pty/src/win/mod.rs b/pty/src/win/mod.rs index 0e9007143..696dd6353 100644 --- a/pty/src/win/mod.rs +++ b/pty/src/win/mod.rs @@ -3,6 +3,7 @@ use anyhow::Context as _; use std::io::{Error as IoError, Result as IoResult}; use std::os::windows::io::{AsRawHandle, RawHandle}; use std::pin::Pin; +use std::sync::Mutex; use std::task::{Context, Poll}; use winapi::shared::minwindef::DWORD; use winapi::um::minwinbase::STILL_ACTIVE; @@ -18,13 +19,14 @@ use filedescriptor::OwnedHandle; #[derive(Debug)] pub struct WinChild { - proc: OwnedHandle, + proc: Mutex, } impl WinChild { fn is_complete(&mut self) -> IoResult> { let mut status: DWORD = 0; - let res = unsafe { GetExitCodeProcess(self.proc.as_raw_handle(), &mut status) }; + let proc = self.proc.lock().unwrap().try_clone().unwrap(); + let res = unsafe { GetExitCodeProcess(proc.as_raw_handle(), &mut status) }; if res != 0 { if status == STILL_ACTIVE { Ok(None) @@ -37,7 +39,8 @@ impl WinChild { } fn do_kill(&mut self) -> IoResult<()> { - let res = unsafe { TerminateProcess(self.proc.as_raw_handle(), 1) }; + let proc = self.proc.lock().unwrap().try_clone().unwrap(); + let res = unsafe { TerminateProcess(proc.as_raw_handle(), 1) }; let err = IoError::last_os_error(); if res != 0 { Err(err) @@ -62,11 +65,12 @@ impl Child for WinChild { if let Ok(Some(status)) = self.try_wait() { return Ok(status); } + let proc = self.proc.lock().unwrap().try_clone().unwrap(); unsafe { - WaitForSingleObject(self.proc.as_raw_handle(), INFINITE); + WaitForSingleObject(proc.as_raw_handle(), INFINITE); } let mut status: DWORD = 0; - let res = unsafe { GetExitCodeProcess(self.proc.as_raw_handle(), &mut status) }; + let res = unsafe { GetExitCodeProcess(proc.as_raw_handle(), &mut status) }; if res != 0 { Ok(ExitStatus::with_exit_code(status)) } else { @@ -85,7 +89,9 @@ impl std::future::Future for WinChild { Ok(None) => { struct PassRawHandleToWaiterThread(pub RawHandle); unsafe impl Send for PassRawHandleToWaiterThread {} - let handle = PassRawHandleToWaiterThread(self.proc.as_raw_handle()); + + let proc = self.proc.lock().unwrap().try_clone()?; + let handle = PassRawHandleToWaiterThread(proc.as_raw_handle()); let waker = cx.waker().clone(); std::thread::spawn(move || { diff --git a/pty/src/win/psuedocon.rs b/pty/src/win/psuedocon.rs index 9cd0a235e..4d4c84bb4 100644 --- a/pty/src/win/psuedocon.rs +++ b/pty/src/win/psuedocon.rs @@ -13,6 +13,7 @@ use std::os::windows::io::{AsRawHandle, FromRawHandle}; use std::os::windows::raw::HANDLE; use std::path::Path; use std::ptr; +use std::sync::Mutex; use winapi::shared::minwindef::DWORD; use winapi::shared::winerror::{HRESULT, S_OK}; use winapi::um::handleapi::*; @@ -150,6 +151,8 @@ impl PsuedoCon { let _main_thread = unsafe { OwnedHandle::from_raw_handle(pi.hThread) }; let proc = unsafe { OwnedHandle::from_raw_handle(pi.hProcess) }; - Ok(WinChild { proc }) + Ok(WinChild { + proc: Mutex::new(proc), + }) } }