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

@@ -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<Box<dyn Child + Send>, Error>;
+    fn spawn_command(&self, cmd: CommandBuilder) -> Result<Box<dyn Child + Send + Sync>, Error>;
 }
 
 /// Represents the exit status of a child process.

pty/src/serial.rs

@@ -99,7 +99,7 @@ struct Slave {
 }
 
 impl SlavePty for Slave {
-    fn spawn_command(&self, cmd: CommandBuilder) -> anyhow::Result<Box<dyn Child + Send>> {
+    fn spawn_command(&self, cmd: CommandBuilder) -> anyhow::Result<Box<dyn Child + Send + Sync>> {
         ensure!(
             cmd.is_default_prog(),
             "can only use default prog commands with serial tty implementations"

pty/src/ssh.rs

@@ -195,7 +195,7 @@ struct SshSlave {
 }
 
 impl SlavePty for SshSlave {
-    fn spawn_command(&self, cmd: CommandBuilder) -> anyhow::Result<Box<dyn Child + Send>> {
+    fn spawn_command(&self, cmd: CommandBuilder) -> anyhow::Result<Box<dyn Child + Send + Sync>> {
         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<dyn Child + Send> = Box::new(SshChild {
+            let child: Box<dyn Child + Send + Sync> = Box::new(SshChild {
                 pty: self.pty.clone(),
             });
 

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<Box<dyn Child + Send>, Error> {
+    fn spawn_command(
+        &self,
+        builder: CommandBuilder,
+    ) -> Result<Box<dyn Child + Send + Sync>, Error> {
         Ok(Box::new(self.fd.spawn_command(builder)?))
     }
 }

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<Box<dyn Child + Send>> {
+    fn spawn_command(&self, cmd: CommandBuilder) -> anyhow::Result<Box<dyn Child + Send + Sync>> {
         let inner = self.inner.lock().unwrap();
         let child = inner.con.spawn_command(cmd)?;
         Ok(Box::new(child))

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<OwnedHandle>,
 }
 
 impl WinChild {
     fn is_complete(&mut self) -> IoResult<Option<ExitStatus>> {
         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 || {

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),
+        })
     }
 }