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