Merge pull request #126 from mahkoh/jorth/command-fds

config: allow attaching file descriptors to commands

Cargo.lock

@@ -23,7 +23,7 @@ version = "0.8.7"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "77c3a9648d43b9cd48db467b3f87fdd6e146bcc88ab0180006cef2179fe11d01"
 dependencies = [
- "cfg-if 1.0.0",
+ "cfg-if",
  "getrandom",
  "once_cell",
  "version_check",
@@ -144,7 +144,7 @@ checksum = "2089b7e3f35b9dd2d0ed921ead4f6d318c27680d4a5bd167b3ee120edb105837"
 dependencies = [
  "addr2line",
  "cc",
- "cfg-if 1.0.0",
+ "cfg-if",
  "libc",
  "miniz_oxide",
  "object",
@@ -200,12 +200,6 @@ version = "1.0.86"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "7f9fa1897e4325be0d68d48df6aa1a71ac2ed4d27723887e7754192705350730"
 
-[[package]]
-name = "cfg-if"
-version = "0.1.10"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "4785bdd1c96b2a846b2bd7cc02e86b6b3dbf14e7e53446c4f54c92a361040822"
-
 [[package]]
 name = "cfg-if"
 version = "1.0.0"
@@ -395,7 +389,7 @@ version = "0.2.12"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "190092ea657667030ac6a35e305e62fc4dd69fd98ac98631e5d3a2b1575a12b5"
 dependencies = [
- "cfg-if 1.0.0",
+ "cfg-if",
  "libc",
  "wasi",
 ]
@@ -575,7 +569,7 @@ version = "0.7.4"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "b67380fd3b2fbe7527a606e18729d21c6f3951633d0500574c4dc22d2d638b9f"
 dependencies = [
- "cfg-if 1.0.0",
+ "cfg-if",
  "winapi",
 ]
 
@@ -585,7 +579,7 @@ version = "0.8.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "c571b676ddfc9a8c12f1f3d3085a7b163966a8fd8098a90640953ce5f6170161"
 dependencies = [
- "cfg-if 1.0.0",
+ "cfg-if",
  "windows-sys 0.48.0",
 ]
 
@@ -694,7 +688,7 @@ version = "0.9.9"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "4c42a9226546d68acdd9c0a280d17ce19bfe27a46bf68784e4066115788d008e"
 dependencies = [
- "cfg-if 1.0.0",
+ "cfg-if",
  "libc",
  "redox_syscall",
  "smallvec",
@@ -1016,12 +1010,12 @@ checksum = "1f3ccbac311fea05f86f61904b462b55fb3df8837a366dfc601a0161d0532f20"
 
 [[package]]
 name = "uapi"
-version = "0.2.10"
+version = "0.2.12"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "019450240401d342e2a5bc47f7fbaeb002a38fe18197b83788750d7ffb143274"
+checksum = "651f13cef1d1988a4c73d215c39fec385fc1be4c7eb6daf89822d5021f7029f8"
 dependencies = [
  "cc",
- "cfg-if 0.1.10",
+ "cfg-if",
  "libc",
  "uapi-proc",
 ]
@@ -1070,7 +1064,7 @@ version = "0.2.90"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "b1223296a201415c7fad14792dbefaace9bd52b62d33453ade1c5b5f07555406"
 dependencies = [
- "cfg-if 1.0.0",
+ "cfg-if",
  "wasm-bindgen-macro",
 ]
 

Cargo.toml

@@ -14,7 +14,7 @@ panic = "abort"
 panic = "abort"
 
 [dependencies]
-uapi = "0.2.10"
+uapi = "0.2.12"
 thiserror = "1.0.56"
 ahash = "0.8.7"
 log = { version = "0.4.20", features = ["std"] }

jay-config/Cargo.toml

@@ -10,6 +10,6 @@ bincode = "1.3.3"
 serde = { version = "1.0.196", features = ["derive"] }
 log = "0.4.14"
 futures-util = {  version = "0.3.30", features = ["io"] }
-uapi = "0.2.10"
+uapi = "0.2.12"
 thiserror = "1.0.57"
 backtrace = "0.3.69"

jay-config/src/_private/client.rs

@@ -28,6 +28,7 @@ use {
         future::Future,
         mem,
         ops::Deref,
+        os::fd::IntoRawFd,
         panic::{catch_unwind, AssertUnwindSafe},
         pin::Pin,
         ptr,
@@ -264,11 +265,26 @@ impl Client {
             .iter()
             .map(|(a, b)| (a.to_string(), b.to_string()))
             .collect();
-        self.send(&ClientMessage::Run {
+        let fds: Vec<_> = command
-            prog: &command.prog,
+            .fds
-            args: command.args.clone(),
+            .borrow_mut()
-            env,
+            .drain()
-        });
+            .map(|(a, b)| (a, b.into_raw_fd()))
+            .collect();
+        if fds.is_empty() {
+            self.send(&ClientMessage::Run {
+                prog: &command.prog,
+                args: command.args.clone(),
+                env,
+            });
+        } else {
+            self.send(&ClientMessage::Run2 {
+                prog: &command.prog,
+                args: command.args.clone(),
+                env,
+                fds,
+            });
+        }
     }
 
     pub fn grab(&self, kb: InputDevice, grab: bool) {

jay-config/src/_private/ipc.rs

@@ -375,6 +375,12 @@ pub enum ClientMessage<'a> {
         pollable: PollableId,
         writable: bool,
     },
+    Run2 {
+        prog: &'a str,
+        args: Vec<String>,
+        env: Vec<(String, String)>,
+        fds: Vec<(i32, i32)>,
+    },
 }
 
 #[derive(Serialize, Deserialize, Debug)]

jay-config/src/exec.rs

@@ -1,6 +1,6 @@
 //! Tools for spawning programs.
 
-use std::collections::HashMap;
+use std::{cell::RefCell, collections::HashMap, os::fd::OwnedFd};
 
 /// Sets an environment variable.
 ///
@@ -14,6 +14,7 @@ pub struct Command {
     pub(crate) prog: String,
     pub(crate) args: Vec<String>,
     pub(crate) env: HashMap<String, String>,
+    pub(crate) fds: RefCell<HashMap<i32, OwnedFd>>,
 }
 
 impl Command {
@@ -28,6 +29,7 @@ impl Command {
             prog: prog.to_string(),
             args: vec![],
             env: Default::default(),
+            fds: Default::default(),
         }
     }
 
@@ -43,7 +45,39 @@ impl Command {
         self
     }
 
+    /// Sets a file descriptor of the process.
+    ///
+    /// By default, the process starts with exactly stdin, stdout, and stderr open and all
+    /// pointing to `/dev/null`.
+    pub fn fd<F: Into<OwnedFd>>(&mut self, idx: i32, fd: F) -> &mut Self {
+        self.fds.borrow_mut().insert(idx, fd.into());
+        self
+    }
+
+    /// Sets the stdin of the process.
+    ///
+    /// This is equivalent to `fd(0, fd)`.
+    pub fn stdin<F: Into<OwnedFd>>(&mut self, fd: F) -> &mut Self {
+        self.fd(0, fd)
+    }
+
+    /// Sets the stdout of the process.
+    ///
+    /// This is equivalent to `fd(1, fd)`.
+    pub fn stdout<F: Into<OwnedFd>>(&mut self, fd: F) -> &mut Self {
+        self.fd(1, fd)
+    }
+
+    /// Sets the stderr of the process.
+    ///
+    /// This is equivalent to `fd(2, fd)`.
+    pub fn stderr<F: Into<OwnedFd>>(&mut self, fd: F) -> &mut Self {
+        self.fd(2, fd)
+    }
+
     /// Executes the command.
+    ///
+    /// This consumes all attached file descriptors.
     pub fn spawn(&self) {
         get!().spawn(self);
     }

src/config/handler.rs

@@ -51,7 +51,7 @@ use {
     log::Level,
     std::{cell::Cell, ops::Deref, rc::Rc, time::Duration},
     thiserror::Error,
-    uapi::{c, fcntl_dupfd_cloexec},
+    uapi::{c, fcntl_dupfd_cloexec, OwnedFd},
 };
 
 pub(super) struct ConfigProxyHandler {
@@ -999,12 +999,17 @@ impl ConfigProxyHandler {
         prog: &str,
         args: Vec<String>,
         env: Vec<(String, String)>,
+        fds: Vec<(i32, i32)>,
     ) -> Result<(), CphError> {
+        let fds: Vec<_> = fds
+            .into_iter()
+            .map(|(a, b)| (a, Rc::new(OwnedFd::new(b))))
+            .collect();
         let forker = match self.state.forker.get() {
             Some(f) => f,
             _ => return Err(CphError::NoForker),
         };
-        forker.spawn(prog.to_string(), args, env, None);
+        forker.spawn(prog.to_string(), args, env, fds);
         Ok(())
     }
 
@@ -1305,7 +1310,7 @@ impl ConfigProxyHandler {
             ClientMessage::GetSeats => self.handle_get_seats(),
             ClientMessage::RemoveSeat { .. } => {}
             ClientMessage::Run { prog, args, env } => {
-                self.handle_run(prog, args, env).wrn("run")?
+                self.handle_run(prog, args, env, vec![]).wrn("run")?
             }
             ClientMessage::GrabKb { kb, grab } => self.handle_grab(kb, grab).wrn("grab")?,
             ClientMessage::SetColor { colorable, color } => {
@@ -1505,6 +1510,12 @@ impl ConfigProxyHandler {
             ClientMessage::AddInterest { pollable, writable } => self
                 .handle_add_interest(pollable, writable)
                 .wrn("add_interest")?,
+            ClientMessage::Run2 {
+                prog,
+                args,
+                env,
+                fds,
+            } => self.handle_run(prog, args, env, fds).wrn("run")?,
         }
         Ok(())
     }

src/forker.rs

@@ -17,6 +17,7 @@ use {
         },
         xwayland,
     },
+    ahash::AHashMap,
     bincode::Options,
     jay_config::_private::bincode_ops,
     log::Level,
@@ -155,12 +156,18 @@ impl ForkerProxy {
         wmfd: Rc<OwnedFd>,
         waylandfd: Rc<OwnedFd>,
     ) -> Result<(Rc<OwnedFd>, c::pid_t), ForkerError> {
-        self.fds
+        let (prog, args) = xwayland::build_args();
-            .borrow_mut()
+        let env = vec![("WAYLAND_SOCKET".to_string(), "6".to_string())];
-            .extend([stderr, dfd, listenfd, wmfd, waylandfd]);
+        let fds = vec![
-        let id = self.next_id.fetch_add(1);
+            (2, stderr),
-        self.outgoing.push(ServerMessage::Xwayland { id });
+            (3, dfd),
-        self.pidfd(id).await
+            (4, listenfd),
+            (5, wmfd),
+            (6, waylandfd),
+        ];
+        let pidfd_id = self.next_id.fetch_add(1);
+        self.spawn_(prog, args, env, fds, Some(pidfd_id));
+        self.pidfd(pidfd_id).await
     }
 
     pub fn spawn(
@@ -168,17 +175,29 @@ impl ForkerProxy {
         prog: String,
         args: Vec<String>,
         env: Vec<(String, String)>,
-        stderr: Option<Rc<OwnedFd>>,
+        fds: Vec<(i32, Rc<OwnedFd>)>,
     ) {
-        let have_stderr = stderr.is_some();
+        self.spawn_(prog, args, env, fds, None)
-        if let Some(stderr) = stderr {
+    }
-            self.fds.borrow_mut().push(stderr);
+
+    fn spawn_(
+        &self,
+        prog: String,
+        args: Vec<String>,
+        env: Vec<(String, String)>,
+        fds: Vec<(i32, Rc<OwnedFd>)>,
+        pidfd_id: Option<u32>,
+    ) {
+        for (_, fd) in &fds {
+            self.fds.borrow_mut().push(fd.clone());
         }
+        let fds = fds.into_iter().map(|(a, _)| a).collect();
         self.outgoing.push(ServerMessage::Spawn {
             prog,
             args,
             env,
-            stderr: have_stderr,
+            fds,
+            pidfd_id,
         })
     }
 
@@ -272,10 +291,8 @@ enum ServerMessage {
         prog: String,
         args: Vec<String>,
         env: Vec<(String, String)>,
-        stderr: bool,
+        fds: Vec<i32>,
-    },
+        pidfd_id: Option<u32>,
-    Xwayland {
-        id: u32,
     },
 }
 
@@ -372,9 +389,9 @@ impl Forker {
                 prog,
                 args,
                 env,
-                stderr,
+                fds,
-            } => self.handle_spawn(prog, args, env, stderr, io),
+                pidfd_id,
-            ServerMessage::Xwayland { id } => self.handle_xwayland(io, id),
+            } => self.handle_spawn(prog, args, env, fds, io, pidfd_id),
         }
     }
 
@@ -386,32 +403,20 @@ impl Forker {
         }
     }
 
-    fn handle_xwayland(self: &Rc<Self>, io: &mut IoIn, id: u32) {
-        let stderr = io.pop_fd();
-        let fds = vec![
-            Rc::try_unwrap(io.pop_fd().unwrap()).unwrap(),
-            Rc::try_unwrap(io.pop_fd().unwrap()).unwrap(),
-            Rc::try_unwrap(io.pop_fd().unwrap()).unwrap(),
-            Rc::try_unwrap(io.pop_fd().unwrap()).unwrap(),
-        ];
-        let (prog, args) = xwayland::build_args(&fds);
-        let env = vec![("WAYLAND_SOCKET".to_string(), fds[3].raw().to_string())];
-        self.spawn(prog, args, env, stderr, fds, Some(id));
-    }
-
     fn handle_spawn(
         self: &Rc<Self>,
         prog: String,
         args: Vec<String>,
         env: Vec<(String, String)>,
-        stderr: bool,
+        fds: Vec<i32>,
         io: &mut IoIn,
+        pidfd_id: Option<u32>,
     ) {
-        let stderr = match stderr {
+        let fds = fds
-            true => io.pop_fd(),
+            .into_iter()
-            _ => None,
+            .map(|a| (a, Rc::try_unwrap(io.pop_fd().unwrap()).unwrap()))
-        };
+            .collect();
-        self.spawn(prog, args, env, stderr, vec![], None)
+        self.spawn(prog, args, env, fds, pidfd_id)
     }
 
     fn spawn(
@@ -419,8 +424,7 @@ impl Forker {
         prog: String,
         args: Vec<String>,
         env: Vec<(String, String)>,
-        stderr: Option<Rc<OwnedFd>>,
+        fds: Vec<(i32, OwnedFd)>,
-        fds: Vec<OwnedFd>,
         pidfd_id: Option<u32>,
     ) {
         let (read, mut write) = pipe2(c::O_CLOEXEC).unwrap();
@@ -476,9 +480,13 @@ impl Forker {
             }
             Forked::Child { .. } => {
                 let err = (|| {
-                    if let Some(stderr) = stderr {
+                    if let Some(max_desired) = fds.iter().map(|v| v.0).max() {
-                        uapi::dup2(stderr.raw(), 2).unwrap();
+                        match uapi::fcntl_dupfd_cloexec(write.raw(), max_desired.wrapping_add(1)) {
+                            Ok(new) => write = new,
+                            Err(e) => return Err(SpawnError::Dupfd(e.into())),
+                        }
                     }
+                    let fds = map_fds(fds)?;
                     for fd in fds {
                         let fd = fd.unwrap();
                         let res: Result<_, Errno> = (|| {
@@ -521,6 +529,8 @@ enum SpawnError {
     Exec(#[source] crate::utils::oserror::OsError),
     #[error("Could not unset cloexec flag")]
     Cloexec(#[source] crate::utils::oserror::OsError),
+    #[error("dupfd faild")]
+    Dupfd(#[source] crate::utils::oserror::OsError),
 }
 
 fn setup_fds(mut socket: OwnedFd) -> OwnedFd {
@@ -564,3 +574,35 @@ fn setup_name(name: &str) {
         c::prctl(c::PR_SET_NAME, name.as_ptr());
     }
 }
+
+fn map_fds(fds: Vec<(i32, OwnedFd)>) -> Result<Vec<OwnedFd>, SpawnError> {
+    let mut desired: Vec<_> = fds.iter().map(|v| v.0).collect();
+    desired.sort_by(|a, b| b.cmp(a));
+    let mut existing_to_desired: AHashMap<_, _> = fds.iter().map(|v| (v.1.raw(), v.0)).collect();
+    let mut desired_to_existing: AHashMap<_, _> = fds.into_iter().map(|v| (v.0, v.1)).collect();
+    for desired in desired {
+        let existing = desired_to_existing.get(&desired).unwrap().raw();
+        if existing == desired {
+            continue;
+        }
+        if let Some(conflict_desired) = existing_to_desired.get(&desired).copied() {
+            match uapi::fcntl_dupfd_cloexec(desired, 0) {
+                Ok(new) => {
+                    existing_to_desired.remove(&desired);
+                    existing_to_desired.insert(new.raw(), conflict_desired);
+                    desired_to_existing.insert(conflict_desired, new);
+                }
+                Err(e) => return Err(SpawnError::Dupfd(e.into())),
+            }
+        }
+        match uapi::dup3(existing, desired, c::O_CLOEXEC) {
+            Ok(_) => {
+                existing_to_desired.remove(&existing);
+                existing_to_desired.insert(desired, desired);
+                desired_to_existing.insert(desired, OwnedFd::new(desired));
+            }
+            Err(e) => return Err(SpawnError::Dupfd(e.into())),
+        }
+    }
+    Ok(desired_to_existing.into_values().collect())
+}

src/xwayland.rs

@@ -198,7 +198,7 @@ async fn run(
     Ok(())
 }
 
-pub fn build_args(fds: &[OwnedFd]) -> (String, Vec<String>) {
+pub fn build_args() -> (String, Vec<String>) {
     let prog = "Xwayland".to_string();
     let args = vec![
         "-terminate".to_string(),
@@ -206,11 +206,11 @@ pub fn build_args(fds: &[OwnedFd]) -> (String, Vec<String>) {
         "-verbose".to_string(),
         10.to_string(),
         "-displayfd".to_string(),
-        fds[0].raw().to_string(),
+        "3".to_string(),
         "-listenfd".to_string(),
-        fds[1].raw().to_string(),
+        "4".to_string(),
         "-wm".to_string(),
-        fds[2].raw().to_string(),
+        "5".to_string(),
     ];
     (prog, args)
 }