Skip to content

Commit

Permalink
Rollup merge of #105458 - Ayush1325:blocking_spawn, r=Mark-Simulacrum
Browse files Browse the repository at this point in the history
Allow blocking `Command::output`

### Problem
Currently, `Command::output` is internally implemented using `Command::spawn`. This is problematic because some targets (like UEFI) do not actually support multitasking and thus block while the program is executing. This coupling does not make much sense as `Command::output` is supposed to block until the execution is complete anyway and thus does not need to rely on a non-blocking `Child` or any other intermediate.

### Solution
This PR moves the implementation of `Command::output` to `std::sys`. This means targets can choose to implement only `Command::output` without having to implement `Command::spawn`.

### Additional Information

This was originally conceived when working on rust-lang/rust#100316. Currently, the only target I know about that will benefit from this change is UEFI.

This PR can also be used to implement more efficient `Command::output` since the intermediate `Process` is not actually needed anymore, but that is outside the scope of this PR.

Since this is not a public API change, I'm not sure if an RFC is needed or not.
  • Loading branch information
matthiaskrgr committed Dec 17, 2022
2 parents af0d409 + 1ec160c commit e21e455
Show file tree
Hide file tree
Showing 11 changed files with 77 additions and 6 deletions.
10 changes: 6 additions & 4 deletions std/src/process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,10 @@ impl Read for ChildStdout {
fn is_read_vectored(&self) -> bool {
self.inner.is_read_vectored()
}

fn read_to_end(&mut self, buf: &mut Vec<u8>) -> io::Result<usize> {
self.inner.read_to_end(buf)
}
}

impl AsInner<AnonPipe> for ChildStdout {
Expand Down Expand Up @@ -907,10 +911,8 @@ impl Command {
/// ```
#[stable(feature = "process", since = "1.0.0")]
pub fn output(&mut self) -> io::Result<Output> {
self.inner
.spawn(imp::Stdio::MakePipe, false)
.map(Child::from_inner)
.and_then(|p| p.wait_with_output())
let (status, stdout, stderr) = self.inner.output()?;
Ok(Output { status: ExitStatus(status), stdout, stderr })
}

/// Executes a command as a child process, waiting for it to finish and
Expand Down
4 changes: 4 additions & 0 deletions std/src/sys/unix/pipe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,10 @@ impl AnonPipe {
self.0.is_read_vectored()
}

pub fn read_to_end(&self, buf: &mut Vec<u8>) -> io::Result<usize> {
self.0.read_to_end(buf)
}

pub fn write(&self, buf: &[u8]) -> io::Result<usize> {
self.0.write(buf)
}
Expand Down
5 changes: 5 additions & 0 deletions std/src/sys/unix/process/process_fuchsia.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,11 @@ impl Command {
Ok((Process { handle: Handle::new(process_handle) }, ours))
}

pub fn output(&mut self) -> io::Result<(ExitStatus, Vec<u8>, Vec<u8>)> {
let (proc, pipes) = self.spawn(Stdio::MakePipe, false)?;
crate::sys_common::process::wait_with_output(proc, pipes)
}

pub fn exec(&mut self, default: Stdio) -> io::Error {
if self.saw_nul() {
return io::const_io_error!(
Expand Down
5 changes: 5 additions & 0 deletions std/src/sys/unix/process/process_unix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,11 @@ impl Command {
}
}

pub fn output(&mut self) -> io::Result<(ExitStatus, Vec<u8>, Vec<u8>)> {
let (proc, pipes) = self.spawn(Stdio::MakePipe, false)?;
crate::sys_common::process::wait_with_output(proc, pipes)
}

// Attempts to fork the process. If successful, returns Ok((0, -1))
// in the child, and Ok((child_pid, -1)) in the parent.
#[cfg(not(target_os = "linux"))]
Expand Down
4 changes: 4 additions & 0 deletions std/src/sys/unix/process/process_unsupported.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ impl Command {
unsupported()
}

pub fn output(&mut self) -> io::Result<(ExitStatus, Vec<u8>, Vec<u8>)> {
unsupported()
}

pub fn exec(&mut self, _default: Stdio) -> io::Error {
unsupported_err()
}
Expand Down
5 changes: 5 additions & 0 deletions std/src/sys/unix/process/process_vxworks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,11 @@ impl Command {
}
}

pub fn output(&mut self) -> io::Result<(ExitStatus, Vec<u8>, Vec<u8>)> {
let (proc, pipes) = self.spawn(Stdio::MakePipe, false)?;
crate::sys_common::process::wait_with_output(proc, pipes)
}

pub fn exec(&mut self, default: Stdio) -> io::Error {
let ret = Command::spawn(self, default, false);
match ret {
Expand Down
4 changes: 4 additions & 0 deletions std/src/sys/unsupported/pipe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ impl AnonPipe {
self.0
}

pub fn read_to_end(&self, _buf: &mut Vec<u8>) -> io::Result<usize> {
self.0
}

pub fn write(&self, _buf: &[u8]) -> io::Result<usize> {
self.0
}
Expand Down
4 changes: 4 additions & 0 deletions std/src/sys/unsupported/process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,10 @@ impl Command {
) -> io::Result<(Process, StdioPipes)> {
unsupported()
}

pub fn output(&mut self) -> io::Result<(ExitStatus, Vec<u8>, Vec<u8>)> {
unsupported()
}
}

impl From<AnonPipe> for Stdio {
Expand Down
6 changes: 5 additions & 1 deletion std/src/sys/windows/pipe.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::os::windows::prelude::*;

use crate::ffi::OsStr;
use crate::io::{self, IoSlice, IoSliceMut};
use crate::io::{self, IoSlice, IoSliceMut, Read};
use crate::mem;
use crate::path::Path;
use crate::ptr;
Expand Down Expand Up @@ -261,6 +261,10 @@ impl AnonPipe {
self.inner.is_read_vectored()
}

pub fn read_to_end(&self, buf: &mut Vec<u8>) -> io::Result<usize> {
self.handle().read_to_end(buf)
}

pub fn write(&self, buf: &[u8]) -> io::Result<usize> {
unsafe {
let len = crate::cmp::min(buf.len(), c::DWORD::MAX as usize) as c::DWORD;
Expand Down
5 changes: 5 additions & 0 deletions std/src/sys/windows/process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,11 @@ impl Command {
))
}
}

pub fn output(&mut self) -> io::Result<(ExitStatus, Vec<u8>, Vec<u8>)> {
let (proc, pipes) = self.spawn(Stdio::MakePipe, false)?;
crate::sys_common::process::wait_with_output(proc, pipes)
}
}

impl fmt::Debug for Command {
Expand Down
31 changes: 30 additions & 1 deletion std/src/sys_common/process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@
use crate::collections::BTreeMap;
use crate::env;
use crate::ffi::{OsStr, OsString};
use crate::sys::process::EnvKey;
use crate::io;
use crate::sys::pipe::read2;
use crate::sys::process::{EnvKey, ExitStatus, Process, StdioPipes};

// Stores a set of changes to an environment
#[derive(Clone, Debug)]
Expand Down Expand Up @@ -117,3 +119,30 @@ impl<'a> ExactSizeIterator for CommandEnvs<'a> {
self.iter.is_empty()
}
}

pub fn wait_with_output(
mut process: Process,
mut pipes: StdioPipes,
) -> io::Result<(ExitStatus, Vec<u8>, Vec<u8>)> {
drop(pipes.stdin.take());

let (mut stdout, mut stderr) = (Vec::new(), Vec::new());
match (pipes.stdout.take(), pipes.stderr.take()) {
(None, None) => {}
(Some(out), None) => {
let res = out.read_to_end(&mut stdout);
res.unwrap();
}
(None, Some(err)) => {
let res = err.read_to_end(&mut stderr);
res.unwrap();
}
(Some(out), Some(err)) => {
let res = read2(out, &mut stdout, err, &mut stderr);
res.unwrap();
}
}

let status = process.wait()?;
Ok((status, stdout, stderr))
}

0 comments on commit e21e455

Please sign in to comment.