From a1ba476e688128623554fae658e8fc26168ee489 Mon Sep 17 00:00:00 2001 From: Julian Orth Date: Wed, 6 Mar 2024 19:47:52 +0100 Subject: [PATCH 1/2] forker: unify xwayland and other spawning --- src/config/handler.rs | 2 +- src/forker.rs | 120 +++++++++++++++++++++++++++--------------- src/xwayland.rs | 8 +-- 3 files changed, 83 insertions(+), 47 deletions(-) diff --git a/src/config/handler.rs b/src/config/handler.rs index 22f102e4..86fa9a4a 100644 --- a/src/config/handler.rs +++ b/src/config/handler.rs @@ -1004,7 +1004,7 @@ impl ConfigProxyHandler { Some(f) => f, _ => return Err(CphError::NoForker), }; - forker.spawn(prog.to_string(), args, env, None); + forker.spawn(prog.to_string(), args, env); Ok(()) } diff --git a/src/forker.rs b/src/forker.rs index 2d6f5ffd..3973b89c 100644 --- a/src/forker.rs +++ b/src/forker.rs @@ -17,6 +17,7 @@ use { }, xwayland, }, + ahash::AHashMap, bincode::Options, jay_config::_private::bincode_ops, log::Level, @@ -155,30 +156,42 @@ impl ForkerProxy { wmfd: Rc, waylandfd: Rc, ) -> Result<(Rc, c::pid_t), ForkerError> { - self.fds - .borrow_mut() - .extend([stderr, dfd, listenfd, wmfd, waylandfd]); - let id = self.next_id.fetch_add(1); - self.outgoing.push(ServerMessage::Xwayland { id }); - self.pidfd(id).await + let (prog, args) = xwayland::build_args(); + let env = vec![("WAYLAND_SOCKET".to_string(), "6".to_string())]; + let fds = vec![ + (2, stderr), + (3, dfd), + (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(&self, prog: String, args: Vec, env: Vec<(String, String)>) { + self.spawn_(prog, args, env, vec![], None) } - pub fn spawn( + fn spawn_( &self, prog: String, args: Vec, env: Vec<(String, String)>, - stderr: Option>, + fds: Vec<(i32, Rc)>, + pidfd_id: Option, ) { - let have_stderr = stderr.is_some(); - if let Some(stderr) = stderr { - self.fds.borrow_mut().push(stderr); + 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 +285,8 @@ enum ServerMessage { prog: String, args: Vec, env: Vec<(String, String)>, - stderr: bool, - }, - Xwayland { - id: u32, + fds: Vec, + pidfd_id: Option, }, } @@ -372,9 +383,9 @@ impl Forker { prog, args, env, - stderr, - } => self.handle_spawn(prog, args, env, stderr, io), - ServerMessage::Xwayland { id } => self.handle_xwayland(io, id), + fds, + pidfd_id, + } => self.handle_spawn(prog, args, env, fds, io, pidfd_id), } } @@ -386,32 +397,20 @@ impl Forker { } } - fn handle_xwayland(self: &Rc, 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, prog: String, args: Vec, env: Vec<(String, String)>, - stderr: bool, + fds: Vec, io: &mut IoIn, + pidfd_id: Option, ) { - let stderr = match stderr { - true => io.pop_fd(), - _ => None, - }; - self.spawn(prog, args, env, stderr, vec![], None) + let fds = fds + .into_iter() + .map(|a| (a, Rc::try_unwrap(io.pop_fd().unwrap()).unwrap())) + .collect(); + self.spawn(prog, args, env, fds, pidfd_id) } fn spawn( @@ -419,8 +418,7 @@ impl Forker { prog: String, args: Vec, env: Vec<(String, String)>, - stderr: Option>, - fds: Vec, + fds: Vec<(i32, OwnedFd)>, pidfd_id: Option, ) { let (read, mut write) = pipe2(c::O_CLOEXEC).unwrap(); @@ -476,9 +474,13 @@ impl Forker { } Forked::Child { .. } => { let err = (|| { - if let Some(stderr) = stderr { - uapi::dup2(stderr.raw(), 2).unwrap(); + if let Some(max_desired) = fds.iter().map(|v| v.0).max() { + 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 +523,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 +568,35 @@ fn setup_name(name: &str) { c::prctl(c::PR_SET_NAME, name.as_ptr()); } } + +fn map_fds(fds: Vec<(i32, OwnedFd)>) -> Result, 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()) +} diff --git a/src/xwayland.rs b/src/xwayland.rs index 5ecb4ea8..af43e885 100644 --- a/src/xwayland.rs +++ b/src/xwayland.rs @@ -198,7 +198,7 @@ async fn run( Ok(()) } -pub fn build_args(fds: &[OwnedFd]) -> (String, Vec) { +pub fn build_args() -> (String, Vec) { let prog = "Xwayland".to_string(); let args = vec![ "-terminate".to_string(), @@ -206,11 +206,11 @@ pub fn build_args(fds: &[OwnedFd]) -> (String, Vec) { "-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) } From 2037a37c1e3975362d99dc3f92c6d65ecd1c711b Mon Sep 17 00:00:00 2001 From: Julian Orth Date: Wed, 6 Mar 2024 19:44:43 +0100 Subject: [PATCH 2/2] config: allow attaching file descriptors to commands --- Cargo.lock | 26 +++++++++------------- Cargo.toml | 2 +- jay-config/Cargo.toml | 2 +- jay-config/src/_private/client.rs | 26 +++++++++++++++++----- jay-config/src/_private/ipc.rs | 6 ++++++ jay-config/src/exec.rs | 36 ++++++++++++++++++++++++++++++- src/config/handler.rs | 17 ++++++++++++--- src/forker.rs | 10 +++++++-- 8 files changed, 96 insertions(+), 29 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2bfb0b5f..06ca0cbc 100644 --- a/Cargo.lock +++ b/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", ] diff --git a/Cargo.toml b/Cargo.toml index 0f21b60f..98706ef5 100644 --- a/Cargo.toml +++ b/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"] } diff --git a/jay-config/Cargo.toml b/jay-config/Cargo.toml index 15246115..bead5f1c 100644 --- a/jay-config/Cargo.toml +++ b/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" diff --git a/jay-config/src/_private/client.rs b/jay-config/src/_private/client.rs index 67d6b7f2..b23a6c44 100644 --- a/jay-config/src/_private/client.rs +++ b/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 { - prog: &command.prog, - args: command.args.clone(), - env, - }); + let fds: Vec<_> = command + .fds + .borrow_mut() + .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) { diff --git a/jay-config/src/_private/ipc.rs b/jay-config/src/_private/ipc.rs index 15c2be29..f23896cd 100644 --- a/jay-config/src/_private/ipc.rs +++ b/jay-config/src/_private/ipc.rs @@ -375,6 +375,12 @@ pub enum ClientMessage<'a> { pollable: PollableId, writable: bool, }, + Run2 { + prog: &'a str, + args: Vec, + env: Vec<(String, String)>, + fds: Vec<(i32, i32)>, + }, } #[derive(Serialize, Deserialize, Debug)] diff --git a/jay-config/src/exec.rs b/jay-config/src/exec.rs index 6f828b18..f1c2e2ee 100644 --- a/jay-config/src/exec.rs +++ b/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, pub(crate) env: HashMap, + pub(crate) fds: RefCell>, } 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>(&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>(&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>(&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>(&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); } diff --git a/src/config/handler.rs b/src/config/handler.rs index 86fa9a4a..030fc498 100644 --- a/src/config/handler.rs +++ b/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, 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); + 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(()) } diff --git a/src/forker.rs b/src/forker.rs index 3973b89c..b5796923 100644 --- a/src/forker.rs +++ b/src/forker.rs @@ -170,8 +170,14 @@ impl ForkerProxy { self.pidfd(pidfd_id).await } - pub fn spawn(&self, prog: String, args: Vec, env: Vec<(String, String)>) { - self.spawn_(prog, args, env, vec![], None) + pub fn spawn( + &self, + prog: String, + args: Vec, + env: Vec<(String, String)>, + fds: Vec<(i32, Rc)>, + ) { + self.spawn_(prog, args, env, fds, None) } fn spawn_(