Auto merge of #145687 - Qelxiros:fd-passing, r=<try>

add std::os::unix::process::CommandExt::fd

try-job: aarch64-apple
try-job: arm-android
try-job: test-various

Author: rust-bors[bot]

library/std/src/os/unix/process.rs

@@ -213,6 +213,94 @@ pub trait CommandExt: Sealed {
 
     #[unstable(feature = "process_setsid", issue = "105376")]
     fn setsid(&mut self, setsid: bool) -> &mut process::Command;
+
+    /// Pass a file descriptor to a child process.
+    ///
+    /// Getting this right is tricky. It is recommended to provide further information to the child
+    /// process by some other mechanism. This could be an argument confirming file descriptors that
+    /// the child can use, device/inode numbers to allow for sanity checks, or something similar.
+    ///
+    /// If `new_fd` is an open file descriptor and closing it would produce one or more errors,
+    /// those errors will be lost when this function is called. See
+    /// [`man 2 dup`](https://www.man7.org/linux/man-pages/man2/dup.2.html#NOTES) for more information.
+    ///
+    /// ```
+    /// #![feature(command_pass_fds)]
+    ///
+    /// use std::process::{Command, Stdio};
+    /// use std::os::unix::process::CommandExt;
+    /// use std::io::{self, Write};
+    ///
+    /// # fn main() -> io::Result<()> {
+    /// let (pipe_reader, mut pipe_writer) = io::pipe()?;
+    ///
+    /// let fd_num = 123;
+    ///
+    /// let mut cmd = Command::new("cat");
+    /// cmd.arg(format!("/dev/fd/{fd_num}")).stdout(Stdio::piped()).fd(fd_num, pipe_reader);
+    ///
+    /// let mut child = cmd.spawn()?;
+    /// let mut stdout = child.stdout.take().unwrap();
+    ///
+    /// pipe_writer.write_all(b"Hello, world!")?;
+    /// drop(pipe_writer);
+    ///
+    /// child.wait()?;
+    /// assert_eq!(io::read_to_string(&mut stdout)?, "Hello, world!");
+    ///
+    /// # Ok(())
+    /// # }
+    /// ```
+    ///
+    /// If this method is called multiple times with the same `new_fd`, all but one file descriptor
+    /// will be lost.
+    ///
+    /// ```
+    /// #![feature(command_pass_fds)]
+    ///
+    /// use std::process::{Command, Stdio};
+    /// use std::os::unix::process::CommandExt;
+    /// use std::io::{self, Write};
+    ///
+    /// # fn main() -> io::Result<()> {
+    /// let (pipe_reader1, mut pipe_writer1) = io::pipe()?;
+    /// let (pipe_reader2, mut pipe_writer2) = io::pipe()?;
+    ///
+    /// let fd_num = 123;
+    ///
+    /// let mut cmd = Command::new("cat");
+    /// cmd.arg(format!("/dev/fd/{fd_num}"))
+    ///     .stdout(Stdio::piped())
+    ///     .fd(fd_num, pipe_reader1)
+    ///     .fd(fd_num, pipe_reader2);
+    ///
+    /// pipe_writer1.write_all(b"Hello from pipe 1!")?;
+    /// drop(pipe_writer1);
+    ///
+    /// pipe_writer2.write_all(b"Hello from pipe 2!")?;
+    /// drop(pipe_writer2);
+    ///
+    /// let mut child = cmd.spawn()?;
+    /// let mut stdout = child.stdout.take().unwrap();
+    ///
+    /// child.wait()?;
+    /// assert_eq!(io::read_to_string(&mut stdout)?, "Hello from pipe 2!");
+    ///
+    /// # Ok(())
+    /// # }
+    /// ```
+    #[unstable(feature = "command_pass_fds", issue = "144989")]
+    fn fd(&mut self, new_fd: RawFd, old_fd: impl Into<OwnedFd>) -> &mut Self;
+
+    /// Check if the last `spawn` of this command used `posix_spawn`.
+    ///
+    /// Returns `None` if the `Command` hasn't been spawned yet.
+    /// Returns `Some(true)` if the last spawn used [`posix_spawn`].
+    /// Returns `Some(false)` otherwise.
+    ///
+    /// [`posix_spawn`]: https://www.man7.org/linux/man-pages/man3/posix_spawn.3.html
+    #[unstable(feature = "command_pass_fds", issue = "144989")]
+    fn last_spawn_was_posix_spawn(&self) -> Option<bool>;
 }
 
 #[stable(feature = "rust1", since = "1.0.0")]
@@ -268,6 +356,15 @@ impl CommandExt for process::Command {
         self.as_inner_mut().setsid(setsid);
         self
     }
+
+    fn fd(&mut self, new_fd: RawFd, old_fd: impl Into<OwnedFd>) -> &mut Self {
+        self.as_inner_mut().fd(old_fd.into(), new_fd);
+        self
+    }
+
+    fn last_spawn_was_posix_spawn(&self) -> Option<bool> {
+        self.as_inner().get_last_spawn_was_posix_spawn()
+    }
 }
 
 /// Unix-specific extensions to [`process::ExitStatus`] and

library/std/src/sys/process/unix/common.rs

@@ -103,6 +103,11 @@ pub struct Command {
     create_pidfd: bool,
     pgroup: Option<pid_t>,
     setsid: bool,
+    // A map of parent FDs to child FDs to be inherited during spawn.
+    fds: Vec<(OwnedFd, RawFd)>,
+    // For testing purposes: store `Some(true)` if the last spawn used `posix_spawn`, `Some(false)`
+    // if it used `exec`, and `None` if it hasn't been spawned yet.
+    last_spawn_was_posix_spawn: Option<bool>,
 }
 
 // passed to do_exec() with configuration of what the child stdio should look
@@ -183,6 +188,8 @@ impl Command {
             create_pidfd: false,
             pgroup: None,
             setsid: false,
+            fds: Vec::new(),
+            last_spawn_was_posix_spawn: None,
         }
     }
 
@@ -360,6 +367,27 @@ impl Command {
         let theirs = ChildPipes { stdin: their_stdin, stdout: their_stdout, stderr: their_stderr };
         Ok((ours, theirs))
     }
+
+    pub fn fd(&mut self, old_fd: OwnedFd, new_fd: RawFd) {
+        self.fds.push((old_fd, new_fd));
+    }
+
+    pub fn get_fds(&self) -> &[(OwnedFd, RawFd)] {
+        &self.fds
+    }
+
+    /// Clear the fd vector, closing all descriptors owned by this `Command`.
+    pub fn close_owned_fds(&mut self) {
+        self.fds.clear();
+    }
+
+    pub fn last_spawn_was_posix_spawn(&mut self, val: bool) {
+        self.last_spawn_was_posix_spawn = Some(val);
+    }
+
+    pub fn get_last_spawn_was_posix_spawn(&self) -> Option<bool> {
+        self.last_spawn_was_posix_spawn
+    }
 }
 
 fn os2c(s: &OsStr, saw_nul: &mut bool) -> CString {

library/std/src/sys/process/unix/unix.rs

@@ -13,6 +13,7 @@ use libc::{gid_t, uid_t};
 use super::common::*;
 use crate::io::{self, Error, ErrorKind};
 use crate::num::NonZero;
+use crate::os::fd::AsRawFd;
 use crate::process::StdioPipes;
 use crate::sys::cvt;
 #[cfg(target_os = "linux")]
@@ -71,8 +72,12 @@ impl Command {
         let (ours, theirs) = self.setup_io(default, needs_stdin)?;
 
         if let Some(ret) = self.posix_spawn(&theirs, envp.as_ref())? {
+            self.last_spawn_was_posix_spawn(true);
+            // Close fds in the parent that have been duplicated in the child
+            self.close_owned_fds();
             return Ok((ret, ours));
         }
+        self.last_spawn_was_posix_spawn(false);
 
         #[cfg(target_os = "linux")]
         let (input, output) = sys::net::Socket::new_pair(libc::AF_UNIX, libc::SOCK_SEQPACKET)?;
@@ -124,6 +129,9 @@ impl Command {
         drop(env_lock);
         drop(output);
 
+        // Close fds in the parent that have been duplicated in the child
+        self.close_owned_fds();
+
         #[cfg(target_os = "linux")]
         let pidfd = if self.get_create_pidfd() { self.recv_pidfd(&input) } else { -1 };
 
@@ -292,6 +300,11 @@ impl Command {
             cvt_r(|| libc::dup2(fd, libc::STDERR_FILENO))?;
         }
 
+        for &(ref old_fd, new_fd) in self.get_fds() {
+            cvt_r(|| libc::dup2(old_fd.as_raw_fd(), new_fd))?;
+            cvt_r(|| libc::close(old_fd.as_raw_fd()))?;
+        }
+
         #[cfg(not(target_os = "l4re"))]
         {
             if let Some(_g) = self.get_groups() {
@@ -455,6 +468,7 @@ impl Command {
         use core::sync::atomic::{Atomic, AtomicU8, Ordering};
 
         use crate::mem::MaybeUninit;
+        use crate::os::fd::AsRawFd;
         use crate::sys::{self, cvt_nz, on_broken_pipe_flag_used};
 
         if self.get_gid().is_some()
@@ -717,6 +731,17 @@ impl Command {
                     libc::STDERR_FILENO,
                 ))?;
             }
+            for &(ref old_fd, new_fd) in self.get_fds() {
+                cvt_nz(libc::posix_spawn_file_actions_adddup2(
+                    file_actions.0.as_mut_ptr(),
+                    old_fd.as_raw_fd(),
+                    new_fd,
+                ))?;
+                cvt_nz(libc::posix_spawn_file_actions_addclose(
+                    file_actions.0.as_mut_ptr(),
+                    old_fd.as_raw_fd(),
+                ))?;
+            }
             if let Some((f, cwd)) = addchdir {
                 cvt_nz(f(file_actions.0.as_mut_ptr(), cwd.as_ptr()))?;
             }

library/std/src/sys/process/unix/unix/tests.rs

@@ -1,10 +1,17 @@
+use crate::fs;
+use crate::os::unix::fs::MetadataExt;
 use crate::os::unix::process::{CommandExt, ExitStatusExt};
 use crate::panic::catch_unwind;
 use crate::process::Command;
 
 // Many of the other aspects of this situation, including heap alloc concurrency
 // safety etc., are tested in tests/ui/process/process-panic-after-fork.rs
 
+/// Use dev + ino to uniquely identify a file
+fn md_file_id(md: &fs::Metadata) -> (u64, u64) {
+    (md.dev(), md.ino())
+}
+
 #[test]
 fn exitstatus_display_tests() {
     // In practice this is the same on every Unix.
@@ -74,3 +81,149 @@ fn test_command_fork_no_unwind() {
             || signal == libc::SIGSEGV
     );
 }
+
+/// For `Command`'s fd-related tests, we want to be sure they work both with exec
+/// and with `posix_spawn`. We test both the default which should use `posix_spawn`
+/// on supported platforms, and using `pre_exec` to force spawn using `exec`.
+mod fd_impls {
+    use super::{assert_spawn_method, md_file_id};
+    use crate::fs;
+    use crate::io::{self, Write};
+    use crate::os::fd::AsRawFd;
+    use crate::os::unix::process::CommandExt;
+    use crate::process::{Command, Stdio};
+
+    /// Check setting the child's stdin via `.fd`.
+    pub fn test_stdin(use_exec: bool) {
+        let (pipe_reader, mut pipe_writer) = io::pipe().unwrap();
+
+        let fd_num = libc::STDIN_FILENO;
+
+        let mut cmd = Command::new("cat");
+        cmd.stdout(Stdio::piped()).fd(fd_num, pipe_reader);
+
+        if use_exec {
+            unsafe {
+                cmd.pre_exec(|| Ok(()));
+            }
+        }
+
+        let mut child = cmd.spawn().unwrap();
+        let mut stdout = child.stdout.take().unwrap();
+
+        assert_spawn_method(&cmd, use_exec);
+
+        pipe_writer.write_all(b"Hello, world!").unwrap();
+        drop(pipe_writer);
+
+        child.wait().unwrap();
+        assert_eq!(io::read_to_string(&mut stdout).unwrap(), "Hello, world!");
+    }
+
+    /// Check that the last `.fd` mapping is preserved when there are conflicts.
+    pub fn test_swap(use_exec: bool) {
+        let (pipe_reader1, mut pipe_writer1) = io::pipe().unwrap();
+        let (pipe_reader2, mut pipe_writer2) = io::pipe().unwrap();
+
+        let num1 = pipe_reader1.as_raw_fd();
+        let num2 = pipe_reader2.as_raw_fd();
+
+        let mut cmd = Command::new("cat");
+        cmd.arg(format!("/dev/fd/{num1}"))
+            .arg(format!("/dev/fd/{num2}"))
+            .stdout(Stdio::piped())
+            .fd(num2, pipe_reader1)
+            .fd(num1, pipe_reader2);
+
+        if use_exec {
+            unsafe {
+                cmd.pre_exec(|| Ok(()));
+            }
+        }
+
+        pipe_writer1.write_all(b"Hello from pipe 1!").unwrap();
+        drop(pipe_writer1);
+
+        pipe_writer2.write_all(b"Hello from pipe 2!").unwrap();
+        drop(pipe_writer2);
+
+        let mut child = cmd.spawn().unwrap();
+        let mut stdout = child.stdout.take().unwrap();
+
+        assert_spawn_method(&cmd, use_exec);
+
+        child.wait().unwrap();
+        // the second pipe's output is clobbered; this is expected.
+        assert_eq!(io::read_to_string(&mut stdout).unwrap(), "Hello from pipe 1!");
+    }
+
+    // ensure that the fd is properly closed in the parent, but only after the child is spawned.
+    pub fn test_close_time(use_exec: bool) {
+        let (_pipe_reader, pipe_writer) = io::pipe().unwrap();
+
+        let fd = pipe_writer.as_raw_fd();
+        let fd_path = format!("/dev/fd/{fd}");
+
+        let mut cmd = Command::new("true");
+        cmd.fd(123, pipe_writer);
+
+        if use_exec {
+            unsafe {
+                cmd.pre_exec(|| Ok(()));
+            }
+        }
+
+        // Get the identifier of the fd (metadata follows symlinks)
+        let fd_id = md_file_id(&fs::metadata(&fd_path).expect("fd should be open"));
+
+        cmd.spawn().unwrap().wait().unwrap();
+
+        assert_spawn_method(&cmd, use_exec);
+
+        // After the child is spawned, our fd should be closed
+        match fs::metadata(&fd_path) {
+            // Ok; fd exists but points to a different file
+            Ok(md) => assert_ne!(md_file_id(&md), fd_id),
+            // Ok; fd does not exist
+            Err(_) => (),
+        }
+    }
+}
+
+#[test]
+fn fd_test_stdin() {
+    fd_impls::test_stdin(false);
+    fd_impls::test_stdin(true);
+}
+
+#[test]
+fn fd_test_swap() {
+    fd_impls::test_swap(false);
+    fd_impls::test_swap(true);
+}
+
+#[test]
+fn fd_test_close_time() {
+    fd_impls::test_close_time(false);
+    fd_impls::test_close_time(true);
+}
+
+#[track_caller]
+fn assert_spawn_method(cmd: &Command, use_exec: bool) {
+    let used_posix_spawn = cmd.last_spawn_was_posix_spawn().unwrap();
+    if use_exec {
+        assert!(!used_posix_spawn, "posix_spawn used but exec was expected");
+    } else if cfg!(any(
+        target_os = "freebsd",
+        target_os = "illumos",
+        all(target_os = "linux", target_env = "gnu"),
+        all(target_os = "linux", target_env = "musl"),
+        target_os = "nto",
+        target_vendor = "apple",
+        target_os = "cygwin",
+    )) {
+        assert!(used_posix_spawn, "platform supports posix_spawn but it wasn't used");
+    } else {
+        assert!(!used_posix_spawn);
+    }
+}