child_reaper: do not reap when runtime process is created

do so by moving runtime creation into child reaper and keeping a lock to synchronize

Signed-off-by: Peter Hunt <pehunt@redhat.com>

conmon-rs/server/src/child_reaper.rs

@@ -13,14 +13,23 @@ use std::{collections::HashMap, fs::File, io::Write, sync::Arc, sync::Mutex};
 impl ChildReaper {
     pub fn init_reaper(&self) -> Result<()> {
         let children = Arc::clone(&self.children);
+        let wait_lock = Arc::clone(&self.wait_lock);
         thread::spawn(move || {
             loop {
-                let children = Arc::clone(&children);
-                match waitpid(Pid::from_raw(-1), None) {
+                // To prevent an error: "No child processes (os error 10)" when spawning new runtime processes,
+                // we don't call waitpid until the runtime process has finished
+                // Inspired by
+                // https://github.com/kata-containers/kata-containers/blob/828a304883a4/src/agent/src/signal.rs#L27
+                let wait_status = {
+                    let wait_lock = Arc::clone(&wait_lock);
+                    let _lock = wait_lock.lock();
+                    waitpid(Pid::from_raw(-1), None)
+                };
+
+                match wait_status {
                     Ok(status) => {
                         if let WaitStatus::Exited(child_pid, exit_status) = status {
-                            // Immediately spawn a thread to reduce risk of dropping
-                            // a SIGCHLD.
+                            let children = Arc::clone(&children);
                             thread::spawn(move || {
                                 if let Err(e) = Self::reap_child(children, child_pid, exit_status) {
                                     error!("Failed to reap child for pid {}: {}", child_pid, e);
@@ -43,6 +52,29 @@ impl ChildReaper {
         Ok(())
     }
 
+    pub fn create_child<P, I, S>(
+        &self,
+        cmd: P,
+        args: I,
+    ) -> Result<std::process::ExitStatus, std::io::Error>
+    where
+        P: AsRef<std::ffi::OsStr>,
+        I: IntoIterator<Item = S>,
+        S: AsRef<std::ffi::OsStr>,
+    {
+        let mut cmd = std::process::Command::new(cmd);
+        cmd.args(args);
+
+        // To prevent an error: "No child processes (os error 10)",
+        // we prevent our reaper thread from calling waitpid until this child has completed.
+        // Inspired by
+        // https://github.com/kata-containers/kata-containers/blob/828a304883a4/src/agent/src/signal.rs#L27
+        let wait_lock = Arc::clone(&self.wait_lock);
+        let _lock = wait_lock.lock().unwrap();
+
+        cmd.spawn()?.wait()
+    }
+
     fn reap_child(
         locked_map: Arc<Mutex<HashMap<i32, ReapableChild>>>,
         child_pid: Pid,
@@ -94,6 +126,7 @@ impl ChildReaper {
 #[derive(Debug, Default)]
 pub struct ChildReaper {
     children: Arc<Mutex<HashMap<i32, ReapableChild>>>,
+    wait_lock: Arc<Mutex<bool>>,
 }
 
 #[derive(Debug, Getters)]

conmon-rs/server/src/rpc.rs

@@ -5,8 +5,7 @@ use capnp_rpc::pry;
 use clap::crate_version;
 use conmon_common::conmon_capnp::conmon;
 use log::debug;
-use std::fs;
-use std::path::PathBuf;
+use std::{fs, path::PathBuf, sync::Arc};
 
 const VERSION: &str = crate_version!();
 
@@ -48,13 +47,10 @@ impl conmon::Server for Server {
 
         let args = pry_err!(self.generate_runtime_args(&params, &maybe_console, &pidfile));
 
-        let mut child = pry!(std::process::Command::new(self.config().runtime())
-            .args(args)
-            .spawn());
+        let child_reaper = Arc::clone(self.reaper());
+        let status = pry_err!(child_reaper.create_child(self.config().runtime(), args));
 
         let id = pry!(req.get_id()).to_string();
-
-        let status = pry!(child.wait());
         debug!("Status for container ID {} is {}", id, status);
         if let Some(console) = maybe_console {
             pry_err!(console
@@ -65,7 +61,7 @@ impl conmon::Server for Server {
         let pid = pry_err!(pry!(fs::read_to_string(pidfile)).parse::<i32>());
         let exit_paths = pry!(path_vec_from_text_list(pry!(req.get_exit_paths())));
 
-        pry_err!(self.reaper().add_child(id, pid, exit_paths));
+        pry_err!(child_reaper.add_child(id, pid, exit_paths));
 
         // TODO FIXME why convert?
         results.get().init_response().set_container_pid(pid as u32);