Fix SSH tunnel not being closed after remote server reboot (#406)

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<String> {
+    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<ConnectionParams, String> {
     let context = params
@@ -311,6 +358,13 @@ pub fn resolve_connection_params(params: &ConnectionParams) -> Result<Connection
 
     let map_key = build_tunnel_map_key(ssh_user, ssh_host, ssh_port, remote_host, remote_port);
 
+    // A tunnel that is no longer alive (e.g. the ssh client exited after the
+    // remote host rebooted) must not be reused: its local port is dead or
+    // still held by the defunct process, which is what produced the "port
+    // already in use" failures on reconnect. Evict it so a fresh tunnel gets
+    // created below.
+    evict_dead_ssh_tunnel(params);
+
     // Check for existing tunnel
     {
         let tunnels = get_tunnels().lock().unwrap();
@@ -2342,6 +2396,26 @@ mod tests {
             assert!(result.is_err());
             assert!(result.unwrap_err().contains("SSH User"));
         }
+
+        #[test]
+        fn test_evict_dead_ssh_tunnel_noop_when_ssh_disabled() {
+            let params = base_params();
+            assert!(!evict_dead_ssh_tunnel(&params));
+        }
+
+        #[test]
+        fn test_evict_dead_ssh_tunnel_noop_without_ssh_fields() {
+            let mut params = create_ssh_params("jump.server", 22, "admin", "db.internal", 3306);
+            params.ssh_user = None;
+            assert!(!evict_dead_ssh_tunnel(&params));
+        }
+
+        #[test]
+        fn test_evict_dead_ssh_tunnel_noop_without_cached_tunnel() {
+            let params =
+                create_ssh_params("no-such-tunnel.host", 22, "admin", "db.internal", 3306);
+            assert!(!evict_dead_ssh_tunnel(&params));
+        }
     }
 
     mod resolve_k8s_params_tests {
@@ -4083,6 +4157,11 @@ pub async fn disconnect_connection<R: Runtime>(
     // Close the connection pool
     crate::pool_manager::close_pool_with_id(&params, 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

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<crate::models::ConnectionParams, String> {
+    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(&params, connection_id)
-            }) {
-                crate::pool_manager::close_pool_with_id(&params, 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.

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::*;