fix(runner): replace perf fifo check with pid check

The FIFO check is very error prone and also slows down the teardown, since we're waiting for the last timeout.

Author: not-matthias

src/executor/shared/fifo.rs

@@ -241,6 +241,7 @@ impl RunnerFifo {
 
             let is_alive = health_check().await?;
             if !is_alive {
+                info!("Process terminated, stopping the command handler");
                 break;
             }
         }

src/executor/wall_time/perf/mod.rs

@@ -31,10 +31,7 @@ use runner_shared::metadata::PerfMetadata;
 use runner_shared::unwind_data::UnwindData;
 use std::path::Path;
 use std::path::PathBuf;
-use std::sync::Arc;
-use std::time::Duration;
 use std::{cell::OnceCell, collections::HashMap, process::ExitStatus};
-use tokio::sync::Mutex;
 
 mod jit_dump;
 mod memory_mappings;
@@ -205,10 +202,12 @@ impl PerfRunner {
         let cmd = wrap_with_sudo(wrapped_builder)?.build();
         debug!("cmd: {cmd:?}");
 
-        let on_process_started = async |_| -> anyhow::Result<()> {
+        let on_process_started = async |pid| -> anyhow::Result<()> {
             // If we output pipedata, we do not parse the perf map during teardown yet, so we need to parse memory
             // maps as we receive the `CurrentBenchmark` fifo commands.
-            let data = Self::handle_fifo(runner_fifo, perf_fifo, self.output_pipedata).await?;
+            let data =
+                Self::handle_fifo(runner_fifo, perf_fifo, self.output_pipedata, pid as pid_t)
+                    .await?;
             self.benchmark_data.set(data).unwrap_or_else(|_| {
                 error!("Failed to set benchmark data in PerfRunner");
             });
@@ -248,37 +247,44 @@ impl PerfRunner {
 
     async fn handle_fifo(
         mut runner_fifo: RunnerFifo,
-        perf_fifo: PerfFifo,
+        mut perf_fifo: PerfFifo,
         parse_memory_maps: bool,
+        pid: pid_t,
     ) -> anyhow::Result<BenchmarkData> {
         let mut symbols_by_pid = HashMap::<pid_t, ProcessSymbols>::new();
         let mut unwind_data_by_pid = HashMap::<pid_t, Vec<UnwindData>>::new();
 
-        let perf_fifo = Arc::new(Mutex::new(perf_fifo));
-        let mut perf_ping_timeout = 5;
+        // The runner spawns a `bash` process, which will execute perf and the benchmark. To check if it
+        // terminated, we have to check if all the sub-processes terminated.
+        // We can't check for the `bash` process, because it will terminate after the FIFO handler has
+        // finished (because we're using a single-threaded runtime).
+        let mut sys = sysinfo::System::new();
         let health_check = async || {
-            let perf_ping = tokio::time::timeout(Duration::from_secs(perf_ping_timeout), async {
-                perf_fifo.lock().await.ping().await
-            })
-            .await;
-            if let Ok(Err(_)) | Err(_) = perf_ping {
-                debug!("Failed to ping perf FIFO, ending perf fifo loop");
-                return Ok(false);
-            }
-            // Perf has started successfully, we can decrease the timeout for future pings
-            perf_ping_timeout = 1;
+            sys.refresh_processes(sysinfo::ProcessesToUpdate::All, true);
+            let process = sys.process(sysinfo::Pid::from_u32(pid as u32));
+
+            match process {
+                None => Ok(false),
+                Some(_proc) => {
+                    let has_children = sys.processes().values().any(|p| {
+                        p.parent()
+                            .map(|parent_pid| parent_pid.as_u32() == pid as u32)
+                            .unwrap_or(false)
+                    });
 
-            Ok(true)
+                    Ok(has_children)
+                }
+            }
         };
 
         let on_cmd = async |cmd: &FifoCommand| {
             #[allow(deprecated)]
             match cmd {
                 FifoCommand::StartBenchmark => {
-                    perf_fifo.lock().await.start_events().await?;
+                    perf_fifo.start_events().await?;
                 }
                 FifoCommand::StopBenchmark => {
-                    perf_fifo.lock().await.stop_events().await?;
+                    perf_fifo.stop_events().await?;
                 }
                 FifoCommand::CurrentBenchmark { pid, .. } => {
                     #[cfg(target_os = "linux")]
@@ -294,7 +300,7 @@ impl PerfRunner {
                     }
                 }
                 FifoCommand::PingPerf => {
-                    if perf_fifo.lock().await.ping().await.is_err() {
+                    if perf_fifo.ping().await.is_err() {
                         return Ok(Some(FifoCommand::Err));
                     }
                     return Ok(Some(FifoCommand::Ack));