Skip to content

Commit

Permalink
Specify process group for child processes
Browse files Browse the repository at this point in the history
To properly close all child processes, they should have the same process group with parent
  • Loading branch information
Alexander Morozov committed Jan 16, 2025
1 parent 7d2bc52 commit 1ce4281
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 24 deletions.
7 changes: 2 additions & 5 deletions src/commands/config/apply.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -36,6 +32,7 @@ fn apply_service_config(
)
.stdout(Stdio::null())
.stdin(Stdio::piped())
.process_group(0)
.spawn()
.context("failed to run picodata admin")?;

Expand Down
2 changes: 2 additions & 0 deletions src/commands/lib.rs
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -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")?;

Expand Down
22 changes: 3 additions & 19 deletions src/commands/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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")?;

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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));
Expand All @@ -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")?;
};

Expand All @@ -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")?;
Expand Down
3 changes: 3 additions & 0 deletions tests/helpers/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -131,6 +132,7 @@ where
.arg("pike")
.args(args)
.current_dir(current_dir)
.process_group(0)
.spawn()
}

Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit 1ce4281

Please sign in to comment.