From 220990849047f411a146924842e023907502c745 Mon Sep 17 00:00:00 2001 From: Oliver Stenbom Date: Mon, 28 Aug 2023 11:37:54 +0200 Subject: [PATCH] Better stop cleanup (#23) - Always remove pid files at the end of stop - Swallow more no such process Fixes DO-841 --- linkup-cli/src/background_tunnel.rs | 4 ++-- linkup-cli/src/stop.rs | 35 ++++++++++++----------------- linkup/src/lib.rs | 5 +---- 3 files changed, 17 insertions(+), 27 deletions(-) diff --git a/linkup-cli/src/background_tunnel.rs b/linkup-cli/src/background_tunnel.rs index fe148cc..4eccc99 100644 --- a/linkup-cli/src/background_tunnel.rs +++ b/linkup-cli/src/background_tunnel.rs @@ -11,7 +11,7 @@ use url::Url; use crate::signal::send_sigint; -use crate::stop::stop_tunnel; +use crate::stop::stop_pid_file; use crate::{linkup_file_path, CliError}; use crate::{LINKUP_CLOUDFLARED_PID, LINKUP_LOCALSERVER_PORT}; @@ -133,7 +133,7 @@ fn try_start_tunnel() -> Result { match rx.recv_timeout(Duration::from_secs(TUNNEL_START_WAIT)) { Ok(result) => result, Err(e) => { - stop_tunnel()?; + stop_pid_file(LINKUP_CLOUDFLARED_PID)?; Err(CliError::StartLocalTunnel(format!( "Failed to obtain tunnel URL within {} seconds: {}", TUNNEL_START_WAIT, e diff --git a/linkup-cli/src/stop.rs b/linkup-cli/src/stop.rs index 8941e85..b30cfa6 100644 --- a/linkup-cli/src/stop.rs +++ b/linkup-cli/src/stop.rs @@ -10,21 +10,14 @@ use crate::{ }; pub fn stop() -> Result<(), CliError> { - let local_stopped = match get_pid(LINKUP_LOCALSERVER_PID_FILE) { - Ok(pid) => send_sigint(&pid).map_err(|e| { - CliError::StopErr(format!( - "Could not send SIGINT to local server pid {}: {}", - pid, e - )) - }), - Err(PidError::NoPidFile(_)) => Ok(()), - Err(e) => Err(CliError::StopErr(format!( - "Could not get local server pid: {}", - e - ))), - }; - - let tunnel_stopped = stop_tunnel(); + let local_stopped = stop_pid_file(LINKUP_LOCALSERVER_PID_FILE); + if local_stopped.is_ok() { + let _ = std::fs::remove_file(LINKUP_LOCALSERVER_PID_FILE); + } + let tunnel_stopped = stop_pid_file(LINKUP_CLOUDFLARED_PID); + if tunnel_stopped.is_ok() { + let _ = std::fs::remove_file(LINKUP_CLOUDFLARED_PID); + } let state = get_state()?; for service in &state.services { @@ -48,23 +41,23 @@ pub fn stop() -> Result<(), CliError> { } } -pub fn stop_tunnel() -> Result<(), CliError> { - match get_pid(LINKUP_CLOUDFLARED_PID) { +pub fn stop_pid_file(pid_file: &str) -> Result<(), CliError> { + match get_pid(pid_file) { Ok(pid) => { match send_sigint(&pid) { Ok(_) => Ok(()), // If we're trying to stop it but it's already died, that's fine Err(PidError::NoSuchProcess(_)) => Ok(()), Err(e) => Err(CliError::StopErr(format!( - "Could not send SIGINT to cloudflared pid {}: {}", - pid, e + "Could not send SIGINT to {} pid {}: {}", + pid_file, pid, e ))), } } Err(PidError::NoPidFile(_)) => Ok(()), Err(e) => Err(CliError::StopErr(format!( - "Could not get cloudflared pid: {}", - e + "Could not get {} pid: {}", + pid_file, e ))), } } diff --git a/linkup/src/lib.rs b/linkup/src/lib.rs index 8cfc9cd..f22f41c 100644 --- a/linkup/src/lib.rs +++ b/linkup/src/lib.rs @@ -319,10 +319,7 @@ mod tests { // Origin let mut origin_headers: HashMap = HashMap::new(); - origin_headers.insert( - "origin".to_string(), - format!("http://{}.example.com", name), - ); + origin_headers.insert("origin".to_string(), format!("http://{}.example.com", name)); sessions .get_request_session("example.com".to_string(), origin_headers) .await