fix(ssh): release the socket, listening port and session when a tunnel ends

CHANGELOG.md

@@ -21,6 +21,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- Leaked socket on every SSH Test Connection against a server reached without jump hosts.
+- Leaked listening port, socket and session each time an SSH tunnel died from sleep or a dropped network.
 - Selection highlight missing from part of a long selection after scrolling back up to it.
 - Editor not scrolling to follow a selection extended past the edge of the viewport.
 - Find highlight and the run band covering only the first line of a match that spans several lines.

TablePro/Core/SSH/LibSSH2Tunnel.swift

@@ -27,7 +27,7 @@ internal final class LibSSH2Tunnel: @unchecked Sendable {
 
     private var forwardingTask: Task<Void, Never>?
     private var keepAliveTask: Task<Void, Never>?
-    private let isAlive = OSAllocatedUnfairLock(initialState: true)
+    private let aliveLatch = TeardownLatch()
     private let clientTasks = OSAllocatedUnfairLock(initialState: [Task<Void, Never>]())
 
     /// Serial queue for all libssh2 calls on this tunnel's session.
@@ -96,7 +96,7 @@ internal final class LibSSH2Tunnel: @unchecked Sendable {
     }
 
     var isRunning: Bool {
-        isAlive.withLock { $0 }
+        aliveLatch.isLive
     }
 
     // MARK: - Forwarding
@@ -170,13 +170,20 @@ internal final class LibSSH2Tunnel: @unchecked Sendable {
     // MARK: - Lifecycle
 
     func close() {
-        let wasAlive = isAlive.withLock { alive -> Bool in
-            let was = alive
-            alive = false
-            return was
-        }
-        guard wasAlive else { return }
+        guard consumeAliveLatch() else { return }
+        performTeardown()
+    }
+
+    /// Takes the one-shot alive latch, returning true to exactly one caller.
+    ///
+    /// The latch decides who performs teardown, so every path that consumes it owes the teardown.
+    /// `markDead` used to consume it and only fire `onDeath`, which left `close()` a no-op for the
+    /// rest of the tunnel's life and every resource it held unreleased.
+    private func consumeAliveLatch() -> Bool {
+        aliveLatch.claim()
+    }
 
+    private func performTeardown() {
         // Cancel all tasks so relay loops see isCancelled
         forwardingTask?.cancel()
         keepAliveTask?.cancel()
@@ -234,12 +241,7 @@ internal final class LibSSH2Tunnel: @unchecked Sendable {
     /// and tear down immediately. We avoid closing socketFD or freeing the session
     /// since relay tasks may still reference them; the OS reclaims all resources.
     func closeSync() {
-        let wasAlive = isAlive.withLock { alive -> Bool in
-            let was = alive
-            alive = false
-            return was
-        }
-        guard wasAlive else { return }
+        guard consumeAliveLatch() else { return }
 
         forwardingTask?.cancel()
         keepAliveTask?.cancel()
@@ -262,14 +264,9 @@ internal final class LibSSH2Tunnel: @unchecked Sendable {
     // MARK: - Private
 
     private func markDead() {
-        let wasAlive = isAlive.withLock { alive -> Bool in
-            let was = alive
-            alive = false
-            return was
-        }
-        if wasAlive {
-            onDeath?(connectionId)
-        }
+        guard consumeAliveLatch() else { return }
+        performTeardown()
+        onDeath?(connectionId)
     }
 
     /// Accepts a client on the listening socket. The accept timestamp is taken here, not once
@@ -378,7 +375,7 @@ internal final class LibSSH2Tunnel: @unchecked Sendable {
         let shouldCancel = clientTasks.withLock { tasks -> Bool in
             tasks.removeAll { $0.isCancelled }
             tasks.append(task)
-            return !isAlive.withLock { $0 }
+            return !aliveLatch.isLive
         }
         if shouldCancel {
             task.cancel()

TablePro/Core/SSH/LibSSH2TunnelFactory.swift

@@ -117,7 +117,6 @@ internal enum LibSSH2TunnelFactory {
     private struct AuthenticatedChain {
         let session: OpaquePointer
         let socketFD: Int32
-        let initialSocketFD: Int32
         let jumpHops: [HopInfo]
 
         struct HopInfo {
@@ -269,7 +268,6 @@ internal enum LibSSH2TunnelFactory {
                 return AuthenticatedChain(
                     session: currentSession,
                     socketFD: currentSocketFD,
-                    initialSocketFD: socketFD,
                     jumpHops: jumpHops
                 )
             } catch {
@@ -309,9 +307,7 @@ internal enum LibSSH2TunnelFactory {
     private static func cleanupChain(_ chain: AuthenticatedChain, reason: String) {
         tablepro_libssh2_session_disconnect(chain.session, reason)
         libssh2_session_free(chain.session)
-        if chain.socketFD != chain.initialSocketFD {
-            Darwin.close(chain.socketFD)
-        }
+        Darwin.close(chain.socketFD)
 
         // Clean up jump hops in reverse order:
         // First pass: cancel relays and shutdown sockets to break relay loops

TablePro/Core/SSH/TeardownLatch.swift

@@ -0,0 +1,36 @@
+//
+//  TeardownLatch.swift
+//  TablePro
+//
+
+import Foundation
+import os
+
+/// Names exactly one owner for a teardown that must happen once.
+///
+/// A tunnel can stop for two reasons at once: the app closes it, and its keep-alive notices the
+/// server has gone. Both need to release the same listening socket, session and jump hops, and
+/// neither may do it twice. A bare boolean invites the shape that caused #2474's collateral leak:
+/// one path took the flag to mean "someone else will tear down" and returned, the other took it to
+/// mean "I have already torn down" and also returned, so nothing was ever released and the tunnel
+/// went permanently deaf to close.
+///
+/// `claim()` returns true to exactly one caller for the lifetime of the latch. **Whoever gets true
+/// owes the teardown.** Checking `isLive` never claims.
+struct TeardownLatch: Sendable {
+    private let live = OSAllocatedUnfairLock(initialState: true)
+
+    /// True for the first caller and false for every caller after it, including concurrent ones.
+    func claim() -> Bool {
+        live.withLock { isLive -> Bool in
+            let was = isLive
+            isLive = false
+            return was
+        }
+    }
+
+    /// Whether the thing this latch guards is still running. Observation only; never claims.
+    var isLive: Bool {
+        live.withLock { $0 }
+    }
+}

TableProTests/Core/SSH/TeardownLatchTests.swift

@@ -0,0 +1,56 @@
+//
+//  TeardownLatchTests.swift
+//  TableProTests
+//
+
+import Foundation
+import Testing
+
+@testable import TablePro
+
+/// The latch exists because two paths can decide a tunnel is over at the same moment, and exactly
+/// one of them has to release its listening socket, session and jump hops.
+@Suite("Teardown latch")
+struct TeardownLatchTests {
+    @Test("The first claim wins and every later one loses")
+    func onlyTheFirstClaimWins() {
+        let latch = TeardownLatch()
+        #expect(latch.claim())
+        #expect(!latch.claim())
+        #expect(!latch.claim())
+    }
+
+    @Test("A latch starts live and stops being live once claimed")
+    func claimingEndsTheLifetime() {
+        let latch = TeardownLatch()
+        #expect(latch.isLive)
+        _ = latch.claim()
+        #expect(!latch.isLive)
+    }
+
+    @Test("Observing does not claim")
+    func observingIsNotClaiming() {
+        let latch = TeardownLatch()
+        #expect(latch.isLive)
+        #expect(latch.isLive)
+        #expect(latch.claim(), "reading isLive must leave the claim available")
+    }
+
+    /// The case the tunnel actually hits: the app closes it in the same instant its keep-alive
+    /// notices the server has gone. Exactly one of them owes the teardown, whichever arrives first.
+    @Test("Exactly one of many concurrent claimants wins")
+    func concurrentClaimantsProduceOneWinner() async {
+        for _ in 0..<200 {
+            let latch = TeardownLatch()
+            let winners = await withTaskGroup(of: Bool.self) { group in
+                for _ in 0..<8 {
+                    group.addTask { latch.claim() }
+                }
+                var count = 0
+                for await didWin in group where didWin { count += 1 }
+                return count
+            }
+            #expect(winners == 1)
+        }
+    }
+}