From 1ce428151a71c62d0edac9588124fa32ddc6477c Mon Sep 17 00:00:00 2001
From: Alexander Morozov <a.morozov@picodata.io>
Date: Thu, 16 Jan 2025 16:17:03 +0300
Subject: [PATCH] Specify process group for child processes

To properly close all child processes, they should have the same process group with parent
---
 src/commands/config/apply.rs |  7 ++-----
 src/commands/lib.rs          |  2 ++
 src/commands/run.rs          | 22 +++-------------------
 tests/helpers/mod.rs         |  3 +++
 4 files changed, 10 insertions(+), 24 deletions(-)

diff --git a/src/commands/config/apply.rs b/src/commands/config/apply.rs
index 7b456d5..ef11125 100644
--- a/src/commands/config/apply.rs
+++ b/src/commands/config/apply.rs
@@ -2,11 +2,7 @@ use anyhow::{Context, Result};
 use serde::Deserialize;
 use serde_yaml::Value;
 use std::{
-    collections::HashMap,
-    fs,
-    io::Write,
-    path::Path,
-    process::{Command, Stdio},
+    collections::HashMap, fs, io::Write, os::unix::process::CommandExt, path::Path, process::{Command, Stdio}
 };
 
 fn apply_service_config(
@@ -36,6 +32,7 @@ fn apply_service_config(
             )
             .stdout(Stdio::null())
             .stdin(Stdio::piped())
+            .process_group(0)
             .spawn()
             .context("failed to run picodata admin")?;
 
diff --git a/src/commands/lib.rs b/src/commands/lib.rs
index 1fbcaf6..0559318 100644
--- a/src/commands/lib.rs
+++ b/src/commands/lib.rs
@@ -1,5 +1,6 @@
 use anyhow::{bail, Context, Result};
 use std::io::{BufRead, BufReader, Read};
+use std::os::unix::process::CommandExt;
 use std::process::{Command, Stdio};
 
 pub enum BuildType {
@@ -17,6 +18,7 @@ pub fn cargo_build(build_type: BuildType) -> Result<()> {
     let mut child = Command::new("cargo")
         .args(args)
         .stdout(Stdio::piped())
+        .process_group(0)
         .spawn()
         .context("running cargo build")?;
 
diff --git a/src/commands/run.rs b/src/commands/run.rs
index 63e1573..26a5652 100644
--- a/src/commands/run.rs
+++ b/src/commands/run.rs
@@ -4,6 +4,7 @@ use log::{error, info};
 use serde::Deserialize;
 use std::fs::File;
 use std::io::Write;
+use std::os::unix::process::CommandExt;
 use std::path::Path;
 use std::process::{exit, Child, Command, Stdio};
 use std::sync::{Arc, Mutex, OnceLock};
@@ -114,6 +115,7 @@ fn enable_plugins(
             .arg("admin")
             .arg(admin_soket.to_str().unwrap())
             .stdin(Stdio::piped())
+            .process_group(0)
             .spawn()
             .context("failed to spawn child proccess of picodata admin")?;
 
@@ -151,17 +153,6 @@ fn push_migration_envs_queries(
     }
 }
 
-fn kill_picodata_instances() -> Result<()> {
-    let processes_lock = Arc::clone(&get_picodata_processes());
-    let mut processes = processes_lock.lock().unwrap();
-
-    for mut process in processes.drain(..) {
-        process.kill()?;
-    }
-
-    Ok(())
-}
-
 #[allow(clippy::too_many_arguments)]
 pub fn cluster(
     topology_path: &PathBuf,
@@ -224,6 +215,7 @@ pub fn cluster(
                     "--tier",
                     tier_name,
                 ])
+                .process_group(0)
                 .spawn()
                 .context(format!("failed to start picodata instance: {instance_id}"))?;
             thread::sleep(Duration::from_secs(5));
@@ -242,11 +234,6 @@ pub fn cluster(
 
     if !disable_plugin_install {
         enable_plugins(topology, data_dir, picodata_path, &plugins_dir)
-            .inspect_err(|_| {
-                kill_picodata_instances().unwrap_or_else(|e| {
-                    error!("failed to kill picodata instances: {:#}", e);
-                });
-            })
             .context("failed to enable plugins")?;
     };
 
@@ -270,9 +257,6 @@ pub fn cmd(
         ctrlc::set_handler(move || {
             info!("{}", "received Ctrl+C. Shutting down ...");
 
-            kill_picodata_instances()
-                .unwrap_or_else(|e| error!("failed to kill picodata instances: {:#}", e));
-
             exit(0);
         })
         .context("failed to set Ctrl+c handler")?;
diff --git a/tests/helpers/mod.rs b/tests/helpers/mod.rs
index 37958cc..670da01 100644
--- a/tests/helpers/mod.rs
+++ b/tests/helpers/mod.rs
@@ -2,6 +2,7 @@ use constcat::concat;
 use log::info;
 use std::ffi::OsStr;
 use std::io::{BufRead, BufReader, Write};
+use std::os::unix::process::CommandExt;
 use std::thread;
 use std::{
     fs::{self},
@@ -131,6 +132,7 @@ where
         .arg("pike")
         .args(args)
         .current_dir(current_dir)
+        .process_group(0)
         .spawn()
 }
 
@@ -168,6 +170,7 @@ pub fn await_picodata_admin(timeout: Duration) -> Result<Child, std::io::Error>
             .arg(PLUGIN_DIR.to_string() + "tmp/cluster/i_1/admin.sock")
             .stdin(Stdio::piped())
             .stdout(Stdio::piped())
+            .process_group(0)
             .spawn();
 
         match picodata_admin {