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

@@ -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",

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 }

com.steampowered.SteamOSManager1.xml

@@ -280,12 +280,12 @@
   </interface>
 
   <!--
-      com.steampowered.SteamOSManager1.Process
+      com.steampowered.SteamOSManager1.SubProcess
       @short_description: Interface to control a subprocess
 
       Version available: 8
   -->
-  <interface name="com.steampowered.SteamOSManager1.Process">
+  <interface name="com.steampowered.SteamOSManager1.SubProcess">
       <!--
         Pause the operation
       -->
@@ -311,7 +311,16 @@
       <method name="Kill"/>
       
       <!--
-        Get the exit code of the process after waiting for it to finish if needed.
+        Wait for process to end and get exit code, resuming if paused
+
+        @exit_code The exit code
+      -->
+      <method name="Wait">
+        <arg type="i" name="exit_code" direction="out"/>
+      </method>
+
+      <!--
+        Get the exit code of the process if possible.
 
         @exit_code The exit code
         -->

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
+    process_manager: ProcessManager,
-    next_process: u32,
 }
 
 impl SteamOSManager {
     pub async fn new(connection: Connection) -> Result<Self> {
         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,23 +146,17 @@ impl SteamOSManager {
 
     async fn update_bios(&mut self) -> zbus::fdo::Result<zbus::zvariant::OwnedObjectPath> {
         // Update the bios as needed
-        ProcessManager::get_command_object_path(
+        self.process_manager
-            "/usr/bin/jupiter-biosupdate",
+            .get_command_object_path("/usr/bin/jupiter-biosupdate", &["--auto"], "updating BIOS")
-            &["--auto"],
-            &mut self.connection,
-            &mut self.next_process,
-            "updating BIOS",
-        )
             .await
     }
 
     async fn update_dock(&mut self) -> zbus::fdo::Result<zbus::zvariant::OwnedObjectPath> {
         // Update the dock firmware as needed
-        ProcessManager::get_command_object_path(
+        self.process_manager
+            .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
@@ -171,11 +164,10 @@ impl SteamOSManager {
 
     async fn trim_devices(&mut self) -> zbus::fdo::Result<zbus::zvariant::OwnedObjectPath> {
         // Run steamos-trim-devices script
-        ProcessManager::get_command_object_path(
+        self.process_manager
+            .get_command_object_path(
                 "/usr/lib/hwsupport/trim-devices.sh",
                 &[] as &[String; 0],
-            &mut self.connection,
-            &mut self.next_process,
                 "trimming devices",
             )
             .await
@@ -191,11 +183,10 @@ impl SteamOSManager {
         if !validate {
             args.push("--skip-validation");
         }
-        ProcessManager::get_command_object_path(
+        self.process_manager
+            .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

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<i32>,
 }
 
 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<OsStr>],
-        connection: &mut zbus::Connection,
-        next_process: &mut u32,
         operation_name: &str,
     ) -> zbus::fdo::Result<zbus::zvariant::OwnedObjectPath> {
         // Run the given executable and give back an object path
-        let path = format!("{}{}", PROCESS_PREFIX, next_process);
+        let path = format!("{}{}", PROCESS_PREFIX, self.next_process);
-        *next_process += 1;
+        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<i32, std::io::Error> = 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<i32> {
-        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<OsStr>],
-    ) -> Result<ProcessManager> {
+    ) -> Result<SubProcess> {
         // 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 SubProcess {
-impl ProcessManager {
+    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<i32> {
+        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<i32> {
+    pub async fn wait(&mut self) -> zbus::fdo::Result<i32> {
         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<i32> {
+        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
+        // Sleep gives 0 exit code when done, -1 when we haven't waited for it yet
-        assert_eq!(pause_process.exit_code().await.unwrap(), 0);
+        assert_eq!(pause_process.wait().await.unwrap(), 0);
 
-        assert_eq!(false_process.exit_code().await.unwrap(), 1);
+        assert_eq!(false_process.wait().await.unwrap(), 1);
-        assert_eq!(true_process.exit_code().await.unwrap(), 0);
+        assert_eq!(true_process.wait().await.unwrap(), 0);
     }
 
     #[tokio::test]