fix(kube): address portforward race condition and temp file leak
All checks were successful
Test / frontend-tests (pull_request) Successful in 1m35s
Test / frontend-typecheck (pull_request) Successful in 1m37s
PR Review Automation / review (pull_request) Successful in 5m38s
Test / rust-fmt-check (pull_request) Successful in 14m13s
Test / rust-clippy (pull_request) Successful in 15m37s
Test / rust-tests (pull_request) Successful in 16m54s

- Replace take().expect() in background task with if let Some to handle
  the race where stop_async/close takes the child before the task is
  polled, preventing a task panic instead of a graceful exit
- Add temp kubeconfig cleanup to stop_async, close, and Drop since
  taking the child out of the shared Arc causes the background task to
  return early, skipping its own cleanup branch
This commit is contained in:
Shaun Arman 2026-06-06 20:40:24 -05:00
parent e56a72a31a
commit 3833d604f7

View File

@ -86,11 +86,15 @@ impl PortForwardSession {
let child_for_task = child_arc.clone();
let temp_path_clone = self.temp_kubeconfig_path.clone();
let join_handle = tokio::spawn(async move {
// Take the child from the Arc
let mut child = child_for_task.lock().await.take().expect("Child not set");
// Take the child from the Arc. If None, stop_async/close already took it and will
// handle cleanup — nothing left to do here.
let child_opt = child_for_task.lock().await.take();
let mut child = match child_opt {
Some(c) => c,
None => return,
};
// Wait for the child process to complete
// This is safe because we're in an async context
let result = child.wait().await;
// Clean up temp kubeconfig file after child completes
@ -146,6 +150,12 @@ impl PortForwardSession {
let _ = child.kill().await;
}
}
// Clean up the temp kubeconfig file. Taking the child above causes the background
// task to exit early without reaching its own cleanup branch.
if let Some(ref path) = self.temp_kubeconfig_path {
let _ = std::fs::remove_file(path);
}
}
pub async fn close(&mut self) {
@ -158,8 +168,11 @@ impl PortForwardSession {
}
}
// Temp file cleanup is now handled by the background task after child completes
// We don't need to clean up here since the background task will do it
// Clean up the temp kubeconfig file. Taking the child above causes the background
// task to exit early without reaching its own cleanup branch.
if let Some(ref path) = self.temp_kubeconfig_path {
let _ = std::fs::remove_file(path);
}
}
pub fn set_error(&mut self, error: String) {
@ -180,18 +193,18 @@ impl PortForwardSession {
impl Drop for PortForwardSession {
fn drop(&mut self) {
// Only kill if not already stopped
if self.is_stopped.load(Ordering::SeqCst) {
return;
}
// Kill the child process if it exists
// Note: This is called from sync context, so we can't await
// The Child will be dropped and cleaned up by the background task
// Drop the handle — detaches the background task. Called from sync context so
// we cannot await kill(); the Child inside the task will be dropped by the OS.
self.child_wait_handle = None;
// Temp file cleanup is now handled by the background task after child completes
// We don't need to clean up here since the background task will do it
// Best-effort temp file cleanup on unexpected drop (e.g., panic paths).
if let Some(ref path) = self.temp_kubeconfig_path {
let _ = std::fs::remove_file(path);
}
}
}