From 04b141ae3409c111c65466252e0987e65aab3363 Mon Sep 17 00:00:00 2001 From: Jesse Luehrs Date: Thu, 30 Dec 2021 22:20:13 -0500 Subject: properly handle configuration when calling spawn multiple times --- src/blocking/command.rs | 48 ++++++++------ src/command.rs | 44 ++++++++----- tests/behavior.rs | 164 ++++++++++++++++++++++++++++++++++++++++++++++++ tests/fds.rs | 2 +- tests/fds_async.rs | 2 +- 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, - stdout: Option, - stderr: Option, + stdin: bool, + stdout: bool, + stderr: bool, + pre_exec_set: bool, pre_exec: Option< Box std::io::Result<()> + Send + Sync + 'static>, >, @@ -14,9 +15,10 @@ impl Command { pub fn new>(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>( &mut self, - cfg: Option, + cfg: T, ) -> &mut Self { - self.stdin = cfg.map(Into::into); + self.stdin = true; + self.inner.stdin(cfg); self } pub fn stdout>( &mut self, - cfg: Option, + cfg: T, ) -> &mut Self { - self.stdout = cfg.map(Into::into); + self.stdout = true; + self.inner.stdout(cfg); self } pub fn stderr>( &mut self, - cfg: Option, + 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, - stdout: Option, - stderr: Option, + stdin: bool, + stdout: bool, + stderr: bool, + pre_exec_set: bool, pre_exec: Option< Box std::io::Result<()> + Send + Sync + 'static>, >, @@ -14,9 +15,10 @@ impl Command { pub fn new>(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>( &mut self, - cfg: Option, + cfg: T, ) -> &mut Self { - self.stdin = cfg.map(Into::into); + self.stdin = true; + self.inner.stdin(cfg); self } pub fn stdout>( &mut self, - cfg: Option, + cfg: T, ) -> &mut Self { - self.stdout = cfg.map(Into::into); + self.stdout = true; + self.inner.stdout(cfg); self } pub fn stderr>( &mut self, - cfg: Option, + 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 @@ -61,6 +61,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(); 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(); -- cgit v1.2.3-54-g00ecf