properly handle configuration when calling spawn multiple times

5 files changed, 224 insertions, 36 deletions

diff --git a/src/blocking/command.rs b/src/blocking/command.rs
index ed36443..0fa97ea 100644
--- a/src/blocking/command.rs
+++ b/src/blocking/command.rs
@@ -2,9 +2,10 @@ use std::os::unix::process::CommandExt as _;
 
 pub struct Command {
     inner: std::process::Command,
-    stdin: Option<std::process::Stdio>,
-    stdout: Option<std::process::Stdio>,
-    stderr: Option<std::process::Stdio>,
+    stdin: bool,
+    stdout: bool,
+    stderr: bool,
+    pre_exec_set: bool,
     pre_exec: Option<
         Box<dyn FnMut() -> std::io::Result<()> + Send + Sync + 'static>,
     >,
@@ -14,9 +15,10 @@ impl Command {
     pub fn new<S: AsRef<std::ffi::OsStr>>(program: S) -> Self {
         Self {
             inner: std::process::Command::new(program),
-            stdin: None,
-            stdout: None,
-            stderr: None,
+            stdin: false,
+            stdout: false,
+            stderr: false,
+            pre_exec_set: false,
             pre_exec: None,
         }
     }
@@ -77,25 +79,28 @@ impl Command {
 
     pub fn stdin<T: Into<std::process::Stdio>>(
         &mut self,
-        cfg: Option<T>,
+        cfg: T,
     ) -> &mut Self {
-        self.stdin = cfg.map(Into::into);
+        self.stdin = true;
+        self.inner.stdin(cfg);
         self
     }
 
     pub fn stdout<T: Into<std::process::Stdio>>(
         &mut self,
-        cfg: Option<T>,
+        cfg: T,
     ) -> &mut Self {
-        self.stdout = cfg.map(Into::into);
+        self.stdout = true;
+        self.inner.stdout(cfg);
         self
     }
 
     pub fn stderr<T: Into<std::process::Stdio>>(
         &mut self,
-        cfg: Option<T>,
+        cfg: T,
     ) -> &mut Self {
-        self.stderr = cfg.map(Into::into);
+        self.stderr = true;
+        self.inner.stderr(cfg);
         self
     }
 
@@ -106,13 +111,19 @@ impl Command {
         let pts = pty.pts();
         let (stdin, stdout, stderr) = crate::sys::setup_subprocess(pts)?;
 
-        self.inner.stdin(self.stdin.take().unwrap_or(stdin));
-        self.inner.stdout(self.stdout.take().unwrap_or(stdout));
-        self.inner.stderr(self.stderr.take().unwrap_or(stderr));
+        if !self.stdin {
+            self.inner.stdin(stdin);
+        }
+        if !self.stdout {
+            self.inner.stdout(stdout);
+        }
+        if !self.stderr {
+            self.inner.stderr(stderr);
+        }
 
         let mut session_leader = crate::sys::session_leader(pts);
-        // safe because setsid() is an async-signal-safe function and ioctl()
-        // is a raw syscall (which is inherently async-signal-safe).
+        // Safety: setsid() is an async-signal-safe function and ioctl() is a
+        // raw syscall (which is inherently async-signal-safe).
         if let Some(mut custom) = self.pre_exec.take() {
             unsafe {
                 self.inner.pre_exec(move || {
@@ -121,9 +132,10 @@ impl Command {
                     Ok(())
                 })
             };
-        } else {
+        } else if !self.pre_exec_set {
             unsafe { self.inner.pre_exec(session_leader) };
         }
+        self.pre_exec_set = true;
 
         Ok(self.inner.spawn()?)
     }
diff --git a/src/command.rs b/src/command.rs
index cd1c8a3..00b51e7 100644
--- a/src/command.rs
+++ b/src/command.rs
@@ -2,9 +2,10 @@ use async_process::unix::CommandExt as _;
 
 pub struct Command {
     inner: async_process::Command,
-    stdin: Option<std::process::Stdio>,
-    stdout: Option<std::process::Stdio>,
-    stderr: Option<std::process::Stdio>,
+    stdin: bool,
+    stdout: bool,
+    stderr: bool,
+    pre_exec_set: bool,
     pre_exec: Option<
         Box<dyn FnMut() -> std::io::Result<()> + Send + Sync + 'static>,
     >,
@@ -14,9 +15,10 @@ impl Command {
     pub fn new<S: AsRef<std::ffi::OsStr>>(program: S) -> Self {
         Self {
             inner: async_process::Command::new(program),
-            stdin: None,
-            stdout: None,
-            stderr: None,
+            stdin: false,
+            stdout: false,
+            stderr: false,
+            pre_exec_set: false,
             pre_exec: None,
         }
     }
@@ -77,25 +79,28 @@ impl Command {
 
     pub fn stdin<T: Into<std::process::Stdio>>(
         &mut self,
-        cfg: Option<T>,
+        cfg: T,
     ) -> &mut Self {
-        self.stdin = cfg.map(Into::into);
+        self.stdin = true;
+        self.inner.stdin(cfg);
         self
     }
 
     pub fn stdout<T: Into<std::process::Stdio>>(
         &mut self,
-        cfg: Option<T>,
+        cfg: T,
     ) -> &mut Self {
-        self.stdout = cfg.map(Into::into);
+        self.stdout = true;
+        self.inner.stdout(cfg);
         self
     }
 
     pub fn stderr<T: Into<std::process::Stdio>>(
         &mut self,
-        cfg: Option<T>,
+        cfg: T,
     ) -> &mut Self {
-        self.stderr = cfg.map(Into::into);
+        self.stderr = true;
+        self.inner.stderr(cfg);
         self
     }
 
@@ -106,9 +111,15 @@ impl Command {
         let pts = pty.pts();
         let (stdin, stdout, stderr) = crate::sys::setup_subprocess(pts)?;
 
-        self.inner.stdin(self.stdin.take().unwrap_or(stdin));
-        self.inner.stdout(self.stdout.take().unwrap_or(stdout));
-        self.inner.stderr(self.stderr.take().unwrap_or(stderr));
+        if !self.stdin {
+            self.inner.stdin(stdin);
+        }
+        if !self.stdout {
+            self.inner.stdout(stdout);
+        }
+        if !self.stderr {
+            self.inner.stderr(stderr);
+        }
 
         let mut session_leader = crate::sys::session_leader(pts);
         // Safety: setsid() is an async-signal-safe function and ioctl() is a
@@ -121,9 +132,10 @@ impl Command {
                     Ok(())
                 })
             };
-        } else {
+        } else if !self.pre_exec_set {
             unsafe { self.inner.pre_exec(session_leader) };
         }
+        self.pre_exec_set = true;
 
         Ok(self.inner.spawn()?)
     }
diff --git a/tests/behavior.rs b/tests/behavior.rs
index ef7d54f..d45dd14 100644
--- a/tests/behavior.rs
+++ b/tests/behavior.rs
@@ -62,6 +62,170 @@ fn test_multiple_async() {
 }
 
 #[test]
+fn test_multiple_configured() {
+    use std::io::BufRead as _;
+    use std::os::unix::io::FromRawFd as _;
+
+    let pty = pty_process::blocking::Pty::new().unwrap();
+    pty.resize(pty_process::Size::new(24, 80)).unwrap();
+
+    let (stderr_pipe_r, stderr_pipe_w) = nix::unistd::pipe().unwrap();
+    let mut stderr_pipe_r = std::io::BufReader::new(unsafe {
+        std::fs::File::from_raw_fd(stderr_pipe_r)
+    });
+    let (pre_exec_pipe_r, pre_exec_pipe_w) = nix::unistd::pipe().unwrap();
+    let mut pre_exec_pipe_r = std::io::BufReader::new(unsafe {
+        std::fs::File::from_raw_fd(pre_exec_pipe_r)
+    });
+    let mut cmd = pty_process::blocking::Command::new("perl");
+    cmd.arg("-Esay 'foo'; say STDERR 'foo-stderr'; open my $fh, '>&=3'; say $fh 'foo-3';")
+        .stderr(unsafe { std::process::Stdio::from_raw_fd(stderr_pipe_w) });
+    unsafe {
+        cmd.pre_exec(move || {
+            nix::unistd::dup2(pre_exec_pipe_w, 3)?;
+            nix::fcntl::fcntl(
+                3,
+                nix::fcntl::F_SETFD(nix::fcntl::FdFlag::empty()),
+            )?;
+            Ok(())
+        });
+    }
+    let mut child = cmd.spawn(&pty).unwrap();
+
+    let mut output = helpers::output(&pty);
+    assert_eq!(output.next().unwrap(), "foo\r\n");
+
+    let mut buf = vec![];
+    nix::unistd::alarm::set(5);
+    stderr_pipe_r.read_until(b'\n', &mut buf).unwrap();
+    nix::unistd::alarm::cancel();
+    assert_eq!(std::string::String::from_utf8(buf).unwrap(), "foo-stderr\n");
+
+    let mut buf = vec![];
+    nix::unistd::alarm::set(5);
+    pre_exec_pipe_r.read_until(b'\n', &mut buf).unwrap();
+    nix::unistd::alarm::cancel();
+    assert_eq!(std::string::String::from_utf8(buf).unwrap(), "foo-3\n");
+
+    let status = child.wait().unwrap();
+    assert_eq!(status.code().unwrap(), 0);
+
+    let mut child = cmd.spawn(&pty).unwrap();
+
+    assert_eq!(output.next().unwrap(), "foo\r\n");
+
+    let mut buf = vec![];
+    nix::unistd::alarm::set(5);
+    stderr_pipe_r.read_until(b'\n', &mut buf).unwrap();
+    nix::unistd::alarm::cancel();
+    assert_eq!(std::string::String::from_utf8(buf).unwrap(), "foo-stderr\n");
+
+    let mut buf = vec![];
+    nix::unistd::alarm::set(5);
+    pre_exec_pipe_r.read_until(b'\n', &mut buf).unwrap();
+    nix::unistd::alarm::cancel();
+    assert_eq!(std::string::String::from_utf8(buf).unwrap(), "foo-3\n");
+
+    let status = child.wait().unwrap();
+    assert_eq!(status.code().unwrap(), 0);
+}
+
+#[cfg(feature = "async")]
+#[test]
+fn test_multiple_configured_async() {
+    use async_std::io::prelude::BufReadExt as _;
+    use futures::stream::StreamExt as _;
+    use std::os::unix::io::FromRawFd as _;
+
+    async_std::task::block_on(async {
+        let pty = pty_process::Pty::new().unwrap();
+        pty.resize(pty_process::Size::new(24, 80)).unwrap();
+
+        let (stderr_pipe_r, stderr_pipe_w) = nix::unistd::pipe().unwrap();
+        let mut stderr_pipe_r = async_std::io::BufReader::new(unsafe {
+            async_std::fs::File::from_raw_fd(stderr_pipe_r)
+        });
+        let (pre_exec_pipe_r, pre_exec_pipe_w) = nix::unistd::pipe().unwrap();
+        let mut pre_exec_pipe_r = async_std::io::BufReader::new(unsafe {
+            async_std::fs::File::from_raw_fd(pre_exec_pipe_r)
+        });
+        let mut cmd = pty_process::Command::new("perl");
+        cmd.arg("-Esay 'foo'; say STDERR 'foo-stderr'; open my $fh, '>&=3'; say $fh 'foo-3';")
+            .stderr(unsafe { std::process::Stdio::from_raw_fd(stderr_pipe_w) });
+        unsafe {
+            cmd.pre_exec(move || {
+                nix::unistd::dup2(pre_exec_pipe_w, 3)?;
+                nix::fcntl::fcntl(
+                    3,
+                    nix::fcntl::F_SETFD(nix::fcntl::FdFlag::empty()),
+                )?;
+                Ok(())
+            });
+        }
+        let mut child = cmd.spawn(&pty).unwrap();
+
+        let mut output = helpers::output_async(&pty);
+        assert_eq!(output.next().await.unwrap(), "foo\r\n");
+
+        let mut buf = vec![];
+        async_std::future::timeout(
+            std::time::Duration::from_secs(5),
+            stderr_pipe_r.read_until(b'\n', &mut buf),
+        )
+        .await
+        .unwrap()
+        .unwrap();
+        assert_eq!(
+            std::string::String::from_utf8(buf).unwrap(),
+            "foo-stderr\n"
+        );
+
+        let mut buf = vec![];
+        async_std::future::timeout(
+            std::time::Duration::from_secs(5),
+            pre_exec_pipe_r.read_until(b'\n', &mut buf),
+        )
+        .await
+        .unwrap()
+        .unwrap();
+        assert_eq!(std::string::String::from_utf8(buf).unwrap(), "foo-3\n");
+
+        let status = child.status().await.unwrap();
+        assert_eq!(status.code().unwrap(), 0);
+
+        let mut child = cmd.spawn(&pty).unwrap();
+
+        assert_eq!(output.next().await.unwrap(), "foo\r\n");
+
+        let mut buf = vec![];
+        async_std::future::timeout(
+            std::time::Duration::from_secs(5),
+            stderr_pipe_r.read_until(b'\n', &mut buf),
+        )
+        .await
+        .unwrap()
+        .unwrap();
+        assert_eq!(
+            std::string::String::from_utf8(buf).unwrap(),
+            "foo-stderr\n"
+        );
+
+        let mut buf = vec![];
+        async_std::future::timeout(
+            std::time::Duration::from_secs(5),
+            pre_exec_pipe_r.read_until(b'\n', &mut buf),
+        )
+        .await
+        .unwrap()
+        .unwrap();
+        assert_eq!(std::string::String::from_utf8(buf).unwrap(), "foo-3\n");
+
+        let status = child.status().await.unwrap();
+        assert_eq!(status.code().unwrap(), 0);
+    });
+}
+
+#[test]
 fn test_controlling_terminal() {
     let pty = pty_process::blocking::Pty::new().unwrap();
     pty.resize(pty_process::Size::new(24, 80)).unwrap();
diff --git a/tests/fds.rs b/tests/fds.rs
index 8a6bb19..8b64d3f 100644
--- a/tests/fds.rs
+++ b/tests/fds.rs
@@ -23,7 +23,7 @@ fn test_fds() {
     pty.resize(pty_process::Size::new(24, 80)).unwrap();
     let mut child = pty_process::blocking::Command::new("perl")
         .arg("-Efor my $fd (0..255) { open my $fh, \"<&=$fd\"; print $fd if stat $fh }; say")
-        .stderr(Some(std::process::Stdio::null()))
+        .stderr(std::process::Stdio::null())
         .spawn(&pty)
         .unwrap();
 
diff --git a/tests/fds_async.rs b/tests/fds_async.rs
index 7710789..9e0bc15 100644
--- a/tests/fds_async.rs
+++ b/tests/fds_async.rs
@@ -52,7 +52,7 @@ fn test_fds_async() {
         pty.resize(pty_process::Size::new(24, 80)).unwrap();
         let mut child = pty_process::Command::new("perl")
             .arg("-Efor my $fd (0..255) { open my $fh, \"<&=$fd\"; print $fd if stat $fh }; say")
-            .stderr(Some(std::process::Stdio::null()))
+            .stderr(std::process::Stdio::null())
             .spawn(&pty)
             .unwrap();