From 88ce0ee12376edd6f5aeed9a56e383ec242f5f99 Mon Sep 17 00:00:00 2001 From: Jeremy Whiting Date: Thu, 2 May 2024 21:25:44 -0600 Subject: [PATCH] Change a bit to have separate SubProcess vs ProcessManager. Keep next_process, connection, etc. in ProcessManager instead of SteamOSManager. Also change exit_code to only give the exit code if known. Added wait to do the wait and get the proper exit code on completion. Also added libc::pid_t use in process.rs. --- Cargo.lock | 5 +- Cargo.toml | 1 + com.steampowered.SteamOSManager1.xml | 15 ++- src/manager.rs | 63 +++++------ src/process.rs | 151 +++++++++++++++++---------- 5 files changed, 139 insertions(+), 96 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index bc112d8..1794720 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -588,9 +588,9 @@ checksum = "e2abad23fbc42b3700f2f279844dc832adb2b2eb069b2df918f455c4e18cc646" [[package]] name = "libc" -version = "0.2.153" +version = "0.2.154" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9c198f91728a82281a64e1f4f9eeb25d82cb32a5de251c6bd1b5154d63a8e7bd" +checksum = "ae743338b92ff9146ce83992f766a31066a91a8c84a45e0e9f21e7cf6de6d346" [[package]] name = "linux-raw-sys" @@ -938,6 +938,7 @@ dependencies = [ "anyhow", "clap", "inotify", + "libc", "nix", "tempfile", "tokio", diff --git a/Cargo.toml b/Cargo.toml index 2a9142b..11b8855 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -14,6 +14,7 @@ strip="symbols" anyhow = "1" clap = { version = "4.5", default-features = false, features = ["derive", "help", "std", "usage"] } inotify = { version = "0.10", default-features = false, features = ["stream"] } +libc = "0.2" nix = { version = "0.28", default-features = false, features = ["fs", "signal"] } tokio = { version = "1", default-features = false, features = ["fs", "io-std", "io-util", "macros", "process", "rt-multi-thread", "signal", "sync"] } tokio-stream = { version = "0.1", default-features = false } diff --git a/com.steampowered.SteamOSManager1.xml b/com.steampowered.SteamOSManager1.xml index b500f9c..6904b6d 100644 --- a/com.steampowered.SteamOSManager1.xml +++ b/com.steampowered.SteamOSManager1.xml @@ -280,12 +280,12 @@ - + @@ -311,7 +311,16 @@ + + + + + diff --git a/src/manager.rs b/src/manager.rs index ac34260..992e710 100644 --- a/src/manager.rs +++ b/src/manager.rs @@ -38,18 +38,17 @@ pub struct SteamOSManager { // Whether we should use trace-cmd or not. // True on galileo devices, false otherwise should_trace: bool, - // Used by ProcessManager but need to only have one of these - next_process: u32, + process_manager: ProcessManager, } impl SteamOSManager { pub async fn new(connection: Connection) -> Result { Ok(SteamOSManager { fan_control: FanControl::new(connection.clone()), - connection, wifi_debug_mode: WifiDebugMode::Off, should_trace: variant().await? == HardwareVariant::Galileo, - next_process: 0, + process_manager: ProcessManager::new(connection.clone()), + connection, }) } } @@ -147,38 +146,31 @@ impl SteamOSManager { async fn update_bios(&mut self) -> zbus::fdo::Result { // Update the bios as needed - ProcessManager::get_command_object_path( - "/usr/bin/jupiter-biosupdate", - &["--auto"], - &mut self.connection, - &mut self.next_process, - "updating BIOS", - ) - .await + self.process_manager + .get_command_object_path("/usr/bin/jupiter-biosupdate", &["--auto"], "updating BIOS") + .await } async fn update_dock(&mut self) -> zbus::fdo::Result { // Update the dock firmware as needed - ProcessManager::get_command_object_path( - "/usr/lib/jupiter-dock-updater/jupiter-dock-updater.sh", - &[] as &[String; 0], - &mut self.connection, - &mut self.next_process, - "updating dock", - ) - .await + self.process_manager + .get_command_object_path( + "/usr/lib/jupiter-dock-updater/jupiter-dock-updater.sh", + &[] as &[String; 0], + "updating dock", + ) + .await } async fn trim_devices(&mut self) -> zbus::fdo::Result { // Run steamos-trim-devices script - ProcessManager::get_command_object_path( - "/usr/lib/hwsupport/trim-devices.sh", - &[] as &[String; 0], - &mut self.connection, - &mut self.next_process, - "trimming devices", - ) - .await + self.process_manager + .get_command_object_path( + "/usr/lib/hwsupport/trim-devices.sh", + &[] as &[String; 0], + "trimming devices", + ) + .await } async fn format_device( @@ -191,14 +183,13 @@ impl SteamOSManager { if !validate { args.push("--skip-validation"); } - ProcessManager::get_command_object_path( - "/usr/lib/hwsupport/format-device.sh", - args.as_ref(), - &mut self.connection, - &mut self.next_process, - format!("formatting {device}").as_str(), - ) - .await + self.process_manager + .get_command_object_path( + "/usr/lib/hwsupport/format-device.sh", + args.as_ref(), + format!("formatting {device}").as_str(), + ) + .await } #[zbus(property(emits_changed_signal = "false"))] diff --git a/src/process.rs b/src/process.rs index 3bb63fe..537f5ec 100644 --- a/src/process.rs +++ b/src/process.rs @@ -6,6 +6,7 @@ */ use anyhow::{anyhow, bail, Result}; +use libc::pid_t; use nix::sys::signal; use nix::sys::signal::Signal; use nix::unistd::Pid; @@ -14,80 +15,106 @@ use tokio::process::{Child, Command}; use tracing::error; use zbus::interface; -use crate::{to_zbus_fdo_error}; +use crate::to_zbus_fdo_error; const PROCESS_PREFIX: &str = "/com/steampowered/SteamOSManager1/Process"; pub struct ProcessManager { + // The thing that manages subprocesses. + // Keeps a handle to the zbus connection and + // what the next process id on the bus should be + connection: zbus::Connection, + next_process: u32, +} + +pub struct SubProcess { process: Child, paused: bool, + exit_code: Option, } impl ProcessManager { + pub fn new(conn: zbus::Connection) -> ProcessManager { + ProcessManager { + connection: conn, + next_process: 0, + } + } + pub async fn get_command_object_path( + &mut self, executable: &str, args: &[impl AsRef], - connection: &mut zbus::Connection, - next_process: &mut u32, operation_name: &str, ) -> zbus::fdo::Result { // Run the given executable and give back an object path - let path = format!("{}{}", PROCESS_PREFIX, next_process); - *next_process += 1; + let path = format!("{}{}", PROCESS_PREFIX, self.next_process); + self.next_process += 1; let pm = ProcessManager::run_long_command(executable, args) .await .inspect_err(|message| error!("Error {operation_name}: {message}")) .map_err(to_zbus_fdo_error)?; - connection.object_server().at(path.as_str(), pm).await?; + self.connection + .object_server() + .at(path.as_str(), pm) + .await?; zbus::zvariant::OwnedObjectPath::try_from(path).map_err(to_zbus_fdo_error) } - fn send_signal(&self, signal: nix::sys::signal::Signal) -> Result<()> { - // if !self.processes.contains_key(&id) { - // println!("no process found with id {id}"); - // return Err(anyhow!("No process found with id {id}")); - // } - - let command = &self.process; - let pid: Result = match command.id() { - Some(id) => match id.try_into() { - Ok(raw_pid) => Ok(raw_pid), - Err(message) => { - bail!("Unable to get pid_t from command {message}"); - } - }, - None => { - bail!("Unable to get pid from command, it likely finished running"); - } - }; - signal::kill(Pid::from_raw(pid.unwrap()), signal)?; - Ok(()) - } - - async fn exit_code_internal(&mut self) -> Result { - let status = self.process.wait().await?; - match status.code() { - Some(code) => Ok(code), - None => bail!("Process exited without giving a code somehow."), - } - } - pub async fn run_long_command( executable: &str, args: &[impl AsRef], - ) -> Result { + ) -> Result { // Run the given executable with the given arguments // Return an id that can be used later to pause/cancel/resume as needed let child = Command::new(executable).args(args).spawn()?; - Ok(ProcessManager { + Ok(SubProcess { process: child, paused: false, + exit_code: None, }) } } -#[interface(name = "com.steampowered.SteamOSManager1.ProcessManager")] -impl ProcessManager { +impl SubProcess { + fn send_signal(&self, signal: nix::sys::signal::Signal) -> Result<()> { + let pid = match self.process.id() { + Some(id) => id, + None => { + bail!("Unable to get pid from command, it likely finished running"); + } + }; + let pid: pid_t = match pid.try_into() { + Ok(pid) => pid, + Err(message) => { + bail!("Unable to get pid_t from command {message}"); + } + }; + signal::kill(Pid::from_raw(pid), signal)?; + Ok(()) + } + + async fn exit_code_internal(&mut self) -> Result { + match self.exit_code { + // Just give the exit_code if we have it already. + Some(code) => Ok(code), + None => { + // Otherwise wait for the process + let status = self.process.wait().await?; + match status.code() { + Some(code) => { + self.exit_code = Some(code); + Ok(code) + } + None => bail!("Process exited without giving a code."), + } + } + } + } +} + +#[interface(name = "com.steampowered.SteamOSManager1.SubProcess")] +impl SubProcess { pub async fn pause(&mut self) -> zbus::fdo::Result<()> { if self.paused { return Err(zbus::fdo::Error::Failed("Already paused".to_string())); @@ -117,11 +144,30 @@ impl ProcessManager { self.send_signal(signal::SIGKILL).map_err(to_zbus_fdo_error) } - pub async fn exit_code(&mut self) -> zbus::fdo::Result { + pub async fn wait(&mut self) -> zbus::fdo::Result { if self.paused { - return Err(zbus::fdo::Error::Failed("Process is paused".to_string())); + self.resume().await?; + } + + let code = match self.exit_code_internal().await.map_err(to_zbus_fdo_error) { + Ok(v) => v, + Err(_) => { + return Err(zbus::fdo::Error::Failed( + "Unable to get exit code".to_string(), + )); + } + }; + self.exit_code = Some(code); + Ok(code) + } + + pub async fn exit_code(&mut self) -> zbus::fdo::Result { + match self.exit_code_internal().await { + Ok(i) => Ok(i), + Err(_) => Err(zbus::fdo::Error::Failed( + "Unable to get exit code.".to_string(), + )), } - self.exit_code_internal().await.map_err(to_zbus_fdo_error) } } @@ -191,14 +237,14 @@ mod test { async fn test_process_manager() { let _h = testing::start(); - let mut false_process = ProcessManager::run_long_command("/bin/false", &[""]) + let mut false_process = ProcessManager::run_long_command("/bin/false", &[] as &[String; 0]) .await .unwrap(); - let mut true_process = ProcessManager::run_long_command("/bin/true", &[""]) + let mut true_process = ProcessManager::run_long_command("/bin/true", &[] as &[String; 0]) .await .unwrap(); - let mut pause_process = ProcessManager::run_long_command("/usr/bin/sleep", &["5"]) + let mut pause_process = ProcessManager::run_long_command("/usr/bin/sleep", &["1"]) .await .unwrap(); let _ = pause_process.pause().await; @@ -208,11 +254,6 @@ mod test { zbus::fdo::Error::Failed("Already paused".to_string()) ); - assert_eq!( - pause_process.exit_code().await.unwrap_err(), - zbus::fdo::Error::Failed("Process is paused".to_string()) - ); - let _ = pause_process.resume().await; assert_eq!( @@ -220,11 +261,11 @@ mod test { zbus::fdo::Error::Failed("Not paused".to_string()) ); - // Sleep gives 0 exit code when done - assert_eq!(pause_process.exit_code().await.unwrap(), 0); + // Sleep gives 0 exit code when done, -1 when we haven't waited for it yet + assert_eq!(pause_process.wait().await.unwrap(), 0); - assert_eq!(false_process.exit_code().await.unwrap(), 1); - assert_eq!(true_process.exit_code().await.unwrap(), 0); + assert_eq!(false_process.wait().await.unwrap(), 1); + assert_eq!(true_process.wait().await.unwrap(), 0); } #[tokio::test]