diff --git a/src-tauri/src/commands.rs b/src-tauri/src/commands.rs index 00d62d811..1dc0b099f 100644 --- a/src-tauri/src/commands.rs +++ b/src-tauri/src/commands.rs @@ -220,6 +220,53 @@ fn build_tunnel_map_key( crate::ssh_tunnel::build_tunnel_key(ssh_user, ssh_host, ssh_port, remote_host, remote_port) } +/// Build the tunnel map key from unresolved params, if they describe an SSH +/// tunnel. Expects params that still carry the original SSH/remote fields +/// (i.e. before [`resolve_connection_params`] rewrites host/port to the local +/// forward). +fn ssh_tunnel_key_for(params: &ConnectionParams) -> Option { + if !params.ssh_enabled.unwrap_or(false) { + return None; + } + let ssh_host = params.ssh_host.as_deref()?; + let ssh_user = params.ssh_user.as_deref()?; + Some(build_tunnel_map_key( + ssh_user, + ssh_host, + params.ssh_port.unwrap_or(22), + params.host.as_deref().unwrap_or("localhost"), + params.port.unwrap_or(DEFAULT_MYSQL_PORT), + )) +} + +/// Stop and remove the SSH tunnel associated with these params, if any. +/// +/// Called when a connection is closed (manually or by the health check) so a +/// tunnel does not outlive the connection that owns it. +pub(crate) fn teardown_ssh_tunnel(params: &ConnectionParams) { + if let Some(map_key) = ssh_tunnel_key_for(params) { + crate::ssh_tunnel::remove_tunnel(&map_key); + } +} + +/// Remove the cached SSH tunnel for these params if it is no longer alive +/// (e.g. the remote server rebooted), so the next resolve creates a fresh one. +/// Returns true if a dead tunnel was evicted. +pub(crate) fn evict_dead_ssh_tunnel(params: &ConnectionParams) -> bool { + let Some(map_key) = ssh_tunnel_key_for(params) else { + return false; + }; + let is_dead = { + let tunnels = get_tunnels().lock().unwrap(); + matches!(tunnels.get(&map_key), Some(tunnel) if !tunnel.is_alive()) + }; + if is_dead { + log::warn!("SSH tunnel {} is no longer alive, removing it", map_key); + crate::ssh_tunnel::remove_tunnel(&map_key); + } + is_dead +} + /// Resolve K8s tunnel params synchronously (no saved-connection lookup; uses inline fields only). fn resolve_k8s_params(params: &ConnectionParams) -> Result { let context = params @@ -311,6 +358,13 @@ pub fn resolve_connection_params(params: &ConnectionParams) -> Result( // Close the connection pool crate::pool_manager::close_pool_with_id(¶ms, Some(&connection_id)).await; + // Tear down the SSH tunnel (if any) so it does not linger holding its + // local port after the connection is gone. `expanded_params` still carries + // the original SSH/remote fields (before resolve rewrites host/port). + teardown_ssh_tunnel(&expanded_params); + log::info!( "Successfully disconnected from connection: {}", connection_id diff --git a/src-tauri/src/health_check.rs b/src-tauri/src/health_check.rs index b1e20d0a9..7b7c805d5 100644 --- a/src-tauri/src/health_check.rs +++ b/src-tauri/src/health_check.rs @@ -136,14 +136,27 @@ async fn ping_all_connections(app: &tauri::AppHandle, failure_counts: &mut HashM } } +/// Look up a saved connection and expand its SSH/K8s params (no tunnel resolution). +async fn expand_params( + app: &tauri::AppHandle, + connection_id: &str, +) -> Result { + let saved_conn = crate::commands::find_connection_by_id(app, connection_id)?; + let expanded = crate::commands::expand_ssh_connection_params(app, &saved_conn.params).await?; + crate::commands::expand_k8s_connection_params(app, &expanded).await +} + /// Ping a single connection by resolving its params and calling driver.ping(). async fn ping_single_connection(app: &tauri::AppHandle, connection_id: &str) -> Result<(), String> { - let saved_conn = crate::commands::find_connection_by_id(app, connection_id)?; + let expanded_params = expand_params(app, connection_id).await?; + + // If the SSH tunnel died (e.g. the server rebooted), evict it and fail the + // ping immediately instead of letting param resolution block on rebuilding + // a tunnel toward a server that may still be down. + if crate::commands::evict_dead_ssh_tunnel(&expanded_params) { + return Err("SSH tunnel is no longer alive".into()); + } - let expanded_params = - crate::commands::expand_ssh_connection_params(app, &saved_conn.params).await?; - let expanded_params = - crate::commands::expand_k8s_connection_params(app, &expanded_params).await?; let params = crate::commands::resolve_connection_params_with_id(&expanded_params, connection_id)?; @@ -167,19 +180,17 @@ async fn handle_connection_failure(app: &tauri::AppHandle, connection_id: &str, // Unregister first to prevent further pings. unregister_connection(connection_id).await; - // Close the pool (best-effort — if params can't be resolved the pool stays orphaned - // but will be reclaimed on next connect or app shutdown). - if let Ok(saved_conn) = crate::commands::find_connection_by_id(app, connection_id) { - if let Ok(expanded) = - crate::commands::expand_ssh_connection_params(app, &saved_conn.params).await - { - let expanded = crate::commands::expand_k8s_connection_params(app, &expanded).await; - if let Ok(params) = expanded.and_then(|params| { - crate::commands::resolve_connection_params_with_id(¶ms, connection_id) - }) { - crate::pool_manager::close_pool_with_id(¶ms, Some(connection_id)).await; - } - } + // Close the pool (best-effort — if params can't be expanded the pool stays orphaned + // but will be reclaimed on next connect or app shutdown). Tunnel resolution is + // skipped deliberately: with a connection_id the pool key only depends on + // driver/connection_id/database, and resolving would try to rebuild a tunnel + // toward a server that may still be down. + if let Ok(expanded) = expand_params(app, connection_id).await { + // Tear down the SSH tunnel before it is reused: after the remote host + // dies the tunnel is dead but still holds its local port, which breaks + // the next reconnect ("port already in use"). + crate::commands::teardown_ssh_tunnel(&expanded); + crate::pool_manager::close_pool_with_id(&expanded, Some(connection_id)).await; } // Notify frontend. diff --git a/src-tauri/src/ssh_tunnel.rs b/src-tauri/src/ssh_tunnel.rs index 1913a1e60..05ac1cb0e 100644 --- a/src-tauri/src/ssh_tunnel.rs +++ b/src-tauri/src/ssh_tunnel.rs @@ -190,6 +190,14 @@ impl SshTunnel { args.push("-o".to_string()); args.push("StrictHostKeyChecking=accept-new".to_string()); + // Keepalives make ssh exit when the server goes away (e.g. a reboot); + // without them the process can hold the local port forever. + args.push("-o".to_string()); + args.push("ServerAliveInterval=15".to_string()); + args.push("-o".to_string()); + args.push("ServerAliveCountMax=3".to_string()); + args.push("-o".to_string()); + args.push("ExitOnForwardFailure=yes".to_string()); args.push("-o".to_string()); if ssh_allow_passphrase_prompt { args.push("BatchMode=no".to_string()); @@ -453,6 +461,15 @@ impl SshTunnel { println!("[SSH Tunnel] Starting tunnel forwarding loop"); while running_clone.load(Ordering::Relaxed) { + // If the SSH session died (e.g. the server rebooted), mark + // the tunnel as dead and release the local port so a + // reconnect can create a fresh tunnel. + if handle.lock().await.is_closed() { + eprintln!("[SSH Tunnel Error] SSH session closed unexpectedly; shutting down tunnel"); + running_clone.store(false, Ordering::Relaxed); + break; + } + let accept = tokio::time::timeout( Duration::from_millis(SSH_ACCEPT_POLL_MS), listener.accept(), @@ -529,10 +546,50 @@ impl SshTunnel { TunnelBackend::SystemSsh(child) => { if let Ok(mut c) = child.lock() { let _ = c.kill(); + // Reap the process so it does not linger as a zombie + // holding the forwarded local port. + let _ = c.wait(); } } } } + + /// Report whether the tunnel is still usable. + /// + /// For a system-ssh backend this detects a process that has already + /// exited (e.g. the ssh client tore down after the remote host rebooted), + /// which would otherwise leave a stale entry in the tunnel map pointing at + /// a dead local port. For the russh backend the `running` flag is cleared + /// either by `stop()` or by the forwarding loop itself when it detects the + /// SSH session has closed. + pub fn is_alive(&self) -> bool { + match &self.backend { + TunnelBackend::Russh(running) => running.load(Ordering::Relaxed), + TunnelBackend::SystemSsh(child) => match child.lock() { + // `Ok(None)` means the child is still running. + Ok(mut c) => matches!(c.try_wait(), Ok(None)), + Err(_) => false, + }, + } + } +} + +/// Stop and remove a tunnel from the global map, if present. +/// +/// Tears down the underlying ssh process / forwarding thread and frees the +/// local port. Safe to call when no tunnel exists for the key. +pub fn remove_tunnel(map_key: &str) { + let tunnel = { + let mut tunnels = get_tunnels().lock().unwrap(); + tunnels.remove(map_key) + }; + if let Some(tunnel) = tunnel { + println!( + "[SSH Tunnel] Removing tunnel '{}' (local port {})", + map_key, tunnel.local_port + ); + tunnel.stop(); + } } /// Test an SSH connection without creating a tunnel @@ -854,6 +911,68 @@ mod tests { } } + mod liveness_tests { + use super::*; + + #[test] + fn russh_is_alive_tracks_running_flag() { + let running = Arc::new(AtomicBool::new(true)); + let tunnel = SshTunnel { + local_port: 0, + backend: TunnelBackend::Russh(running.clone()), + }; + assert!(tunnel.is_alive()); + + // stop() flips the flag; the tunnel is then considered dead. + tunnel.stop(); + assert!(!tunnel.is_alive()); + } + + #[cfg(unix)] + #[test] + fn system_ssh_is_alive_detects_exited_child() { + // A long-running child is alive... + let child = Command::new("sleep") + .arg("30") + .spawn() + .expect("spawn sleep"); + let tunnel = SshTunnel { + local_port: 0, + backend: TunnelBackend::SystemSsh(Arc::new(Mutex::new(child))), + }; + assert!(tunnel.is_alive()); + + // ...and dead once stopped (killed + reaped). + tunnel.stop(); + assert!(!tunnel.is_alive()); + } + + #[test] + fn remove_tunnel_is_noop_for_unknown_key() { + // Must not panic when the key is absent from the map. + remove_tunnel("nonexistent@host:22:remote->3306"); + } + + #[cfg(unix)] + #[test] + fn remove_tunnel_stops_and_removes_entry() { + let key = "removal-test@host:22:remote->3306".to_string(); + let child = Command::new("sleep") + .arg("30") + .spawn() + .expect("spawn sleep"); + let tunnel = SshTunnel { + local_port: 0, + backend: TunnelBackend::SystemSsh(Arc::new(Mutex::new(child))), + }; + get_tunnels().lock().unwrap().insert(key.clone(), tunnel); + + remove_tunnel(&key); + + assert!(!get_tunnels().lock().unwrap().contains_key(&key)); + } + } + mod is_empty_or_whitespace_tests { use super::*;