Skip to content
This repository has been archived by the owner on Dec 21, 2024. It is now read-only.

Commit

Permalink
fix(toolchain): force kill existing pid for process manager not stopp…
Browse files Browse the repository at this point in the history
…ed cleanly (#506)

Fixes GDT-156
  • Loading branch information
NathanFlurry committed Sep 29, 2024
1 parent b2c0388 commit 24ff721
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 3 deletions.
13 changes: 13 additions & 0 deletions packages/toolchain/src/config/meta.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ pub struct Meta {
/// See `backend_port`.
#[serde(default)]
pub editor_port: Option<u16>,

/// See `backend_port`.
#[serde(default)]
pub process_manager_state: HashMap<String, ProcessManagerState>,
}

#[derive(Serialize, Deserialize)]
Expand Down Expand Up @@ -58,6 +62,15 @@ pub struct Backend {
pub db_url: Option<String>,
}

#[derive(Default, Clone, Serialize, Deserialize)]
pub struct ProcessManagerState {
/// Stores the PID of the current child process.
///
/// We use this to kill the existing PID in case the parent process dies without gracefully
/// killing the children.
pub pid: Option<u32>,
}

lazy_static! {
/// List of all meta paths cached in memory.
///
Expand Down
36 changes: 33 additions & 3 deletions packages/toolchain/src/util/process_manager/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use tokio::{
sync::{broadcast, mpsc, watch, Mutex},
};

use crate::util::task::TaskCtx;
use crate::{config::meta, paths, util::task::TaskCtx};

#[cfg(unix)]
mod unix;
Expand Down Expand Up @@ -119,6 +119,17 @@ impl ProcessManager {
where
CommandFut: Future<Output = Result<CommandOpts>>,
{
// Terminate the previous process if needed
if let Some(old_pid) = meta::mutate_project(&paths::data_dir()?, |x| {
x.process_manager_state
.get_mut(self.key)
.and_then(|x| x.pid.take())
})
.await?
{
os::kill_process_tree(old_pid).await?;
}

// Start new process if needed. Otherwise, will hook to the existing process.
//
// Clonign value required since this holds a read lock on the inner
Expand Down Expand Up @@ -175,7 +186,7 @@ impl ProcessManager {

// Re-throw error
if let Some(error) = error {
bail!("process erorr: {error}");
bail!("process error: {error}");
}

Ok(exit_code)
Expand Down Expand Up @@ -269,9 +280,18 @@ impl ProcessManager {

// Spawn the command
let mut child = cmd.spawn()?;
let child_pid = child.id().expect("missing child pid");

// Save the PID
meta::mutate_project(&paths::data_dir()?, |x| {
x.process_manager_state
.entry(self.key.to_string())
.or_default()
.pid = Some(child_pid);
})
.await?;

// Update state
let child_pid = child.id().expect("missing child pid");
self.status_tx
.send(ProcessStatus::Running { pid: child_pid })
.context("send ProcessStatus::Running")?;
Expand Down Expand Up @@ -341,6 +361,16 @@ impl ProcessManager {
stdout_handle.await??;
stderr_handle.await??;

// Remove PID
meta::mutate_project(&paths::data_dir()?, |x| {
x.process_manager_state.get_mut(self.key).map(|x| {
if x.pid == Some(child_pid) {
x.pid = None;
}
})
})
.await?;

// Update state
self.status_tx
.send(ProcessStatus::Exited {
Expand Down

0 comments on commit 24ff721

Please sign in to comment.