From a2366a7ea3bbb7763c62cc9ad39e2d9d72559a99 Mon Sep 17 00:00:00 2001 From: NeoPlays <80448387+NeoPlays@users.noreply.github.com> Date: Thu, 10 Sep 2026 14:47:22 +0200 Subject: [PATCH 1/4] FIX: Prevent the SSH reconnect storm on connection loss and auto-dismiss the reconnect modal --- launcher/src/backend/HetznerServer.js | 2 +- launcher/src/backend/NodeConnection.js | 1 - launcher/src/backend/SSHService.js | 204 +++++++++++++----- .../backend/tests/unit/SSHReconnect.test.js | 195 +++++++++++++++++ launcher/src/background.js | 16 +- .../components/modals/ReconnectModal.vue | 4 +- launcher/src/components/base/BaseLayout.vue | 18 ++ launcher/src/store/nodeHeader.js | 1 + 8 files changed, 381 insertions(+), 60 deletions(-) create mode 100644 launcher/src/backend/tests/unit/SSHReconnect.test.js diff --git a/launcher/src/backend/HetznerServer.js b/launcher/src/backend/HetznerServer.js index fbf7f44b1b..ab1008d555 100755 --- a/launcher/src/backend/HetznerServer.js +++ b/launcher/src/backend/HetznerServer.js @@ -216,7 +216,7 @@ export class HetznerServer { } async finishTestGracefully(nodeConnection) { - clearInterval(nodeConnection.sshService.checkPoolPolling); + nodeConnection.sshService.reconnectAbort?.abort(); await this.Sleep(10000); await nodeConnection.sshService.disconnect(); await this.deleteSSHKey(); diff --git a/launcher/src/backend/NodeConnection.js b/launcher/src/backend/NodeConnection.js index ad21dc6334..3b1bedf741 100755 --- a/launcher/src/backend/NodeConnection.js +++ b/launcher/src/backend/NodeConnection.js @@ -40,7 +40,6 @@ export class NodeConnection { await this.sshService.disconnect(true); } await this.sshService.connect(this.nodeConnectionParams, currentWindow); - this.sshService.addingConnection = true; await this.findStereumSettings(); this.taskManager = taskManager; } catch (error) { diff --git a/launcher/src/backend/SSHService.js b/launcher/src/backend/SSHService.js index 4d23b5f03d..6abee61031 100755 --- a/launcher/src/backend/SSHService.js +++ b/launcher/src/backend/SSHService.js @@ -11,20 +11,129 @@ const log = require("electron-log"); const ping = require("ping"); export class SSHService { - constructor() { + // Connections open lazily, never on a timer: a poll loop can't tell "slow" from "gone" and + // buries sshd in half-open handshakes until MaxStartups/fail2ban lock the client out. + static MAX_POOL_SIZE = 6; + static MAX_SESSIONS_PER_CONNECTION = 5; + static RECONNECT_DELAYS_MS = [2000, 5000, 15000, 30000, 60000]; + static KEEPALIVE_INTERVAL_MS = 10000; + static KEEPALIVE_COUNT_MAX = 3; + static READY_TIMEOUT_MS = 20000; + + constructor(onStateChange = null) { this.connectionPool = []; this.connectionInfo = null; this.connected = false; this.tunnels = []; this.rpcReceivedDatas = []; - this.addingConnection = false; - this.removeConnectionCount = 0; - this.checkPoolPolling = setInterval(async () => { - await this.checkConnectionPool(); - }, 100); this.shellConn = null; this.shellStream = null; this.loggingOut = false; + // "connected" | "reconnecting" | "disconnected", deduplicated + this.onStateChange = onStateChange; + this.lastState = null; + this.reconnecting = false; + this.reconnectAbort = null; + this.growing = null; // in-flight pool growth, shared so concurrent execs open one connection + this.epoch = 0; // bumped on disconnect so a handshake still in flight is discarded, not pooled + } + + emitState(state) { + if (state === this.lastState) return; + this.lastState = state; + try { + this.onStateChange?.(state); + } catch (err) { + log.error("onStateChange listener threw: ", err); + } + } + + sleep(ms, signal) { + return new Promise((resolve, reject) => { + const timer = setTimeout(resolve, ms); + signal.addEventListener( + "abort", + () => { + clearTimeout(timer); + reject(new Error("aborted")); + }, + { once: true } + ); + }); + } + + /** Drop a dead connection. Wired to "error", "end" and "close" so a blackholed transport + * (a VPN switch) can't leave a corpse in the pool that makes the service look connected. */ + dropConnection(conn) { + const before = this.connectionPool.length; + this.connectionPool = this.connectionPool.filter((c) => c !== conn); + if (this.connectionPool.length === before || this.connectionPool.length > 0) return; + + const wasConnected = this.lastState === "connected"; + this.connected = false; + if (this.loggingOut) return; // intentional teardown + if (wasConnected && !this.reconnecting) { + this.reconnectWithBackoff(); + } else if (!this.reconnecting) { + this.emitState("disconnected"); + } + } + + /** Reconnect on the RECONNECT_DELAYS_MS schedule, one attempt in flight at a time. */ + async reconnectWithBackoff() { + this.reconnectAbort?.abort(); + if (this.connectionPool.length > 0) return true; + const abort = new AbortController(); + this.reconnectAbort = abort; + this.reconnecting = true; + this.emitState("reconnecting"); + const delays = SSHService.RECONNECT_DELAYS_MS; + try { + for (let i = 0; i < delays.length; i++) { + try { + await this.sleep(delays[i], abort.signal); + } catch { + return false; // aborted + } + if (this.loggingOut || !this.connectionInfo) return false; + try { + await this.connect(this.connectionInfo); + return true; + } catch (err) { + log.warn(`SSH :: reconnect attempt ${i + 1}/${delays.length} failed: ${err?.message || err}`); + } + } + this.emitState("disconnected"); + return false; + } finally { + if (this.reconnectAbort === abort) { + this.reconnecting = false; + this.reconnectAbort = null; + } + } + } + + /** Pooled connection with spare capacity, opening one if needed. Growth is single-flight. */ + async acquireConnection() { + let conn = this.getConnectionFromPool(); + if (conn) return conn; + if (this.loggingOut || !this.connectionInfo) return null; + if (this.connectionPool.length >= SSHService.MAX_POOL_SIZE) { + // saturated: reuse the least-loaded rather than opening past the cap + return this.connectionPool.reduce((a, b) => (a._chanMgr._count <= b._chanMgr._count ? a : b), this.connectionPool[0]); + } + if (!this.growing) { + this.growing = this.connect(this.connectionInfo) + .catch((err) => { + log.error("Failed opening an SSH connection: ", err); + return null; + }) + .finally(() => { + this.growing = null; + }); + } + await this.growing; + return this.getConnectionFromPool() ?? this.connectionPool[0] ?? null; } static checkExecError(err, accept_empty_result = false) { @@ -87,50 +196,30 @@ export class SSHService { return connectionQuality; } - async checkConnectionPool() { - let lastIndex = this.connectionPool.length - 1; - const threshholdIndex = lastIndex - 2; - - if ( - this.connectionInfo && - this.addingConnection && - (this.connectionPool.length < 6 || this.connectionPool[threshholdIndex]?._chanMgr?._count > 0) && - process.env.NODE_ENV != "test" - ) { - await this.connect(this.connectionInfo); - } - if (this.connectionPool.length > 5 && this.connectionPool[threshholdIndex]?._chanMgr?._count === 0) { - this.removeConnectionCount++; - } else { - this.removeConnectionCount = 0; - } - if (this.removeConnectionCount > 100) { - this.removeConnectionCount = 0; - this.connectionPool.pop().end(); - } - } - + /** Least-loaded connection with spare capacity, or undefined. Never opens one. */ getConnectionFromPool() { - let conn; - let maxVal = 5; - while (!conn && maxVal < 10) { - conn = this.connectionPool.find((c) => c._chanMgr._count < maxVal); - maxVal++; - } - return conn; + const candidates = this.connectionPool.filter((c) => c._chanMgr._count < SSHService.MAX_SESSIONS_PER_CONNECTION); + if (candidates.length === 0) return undefined; + return candidates.reduce((a, b) => (a._chanMgr._count <= b._chanMgr._count ? a : b)); } async connect(connectionInfo, currentWindow = null) { this.connectionInfo = connectionInfo; + const epoch = this.epoch; let conn = new Client(); let passwordBanner = false; return new Promise((resolve, reject) => { conn.on("error", (error) => { - this.addingConnection = false; log.error(error); + this.dropConnection(conn); reject(error); }); - conn.on("close", () => {}); + conn.on("end", () => { + this.dropConnection(conn); + }); + conn.on("close", () => { + this.dropConnection(conn); + }); //only works for ubuntu 22.04 conn.on("banner", (msg) => { if (new RegExp(/^(?=.*\bchange\b)(?=.*\bpassword\b).*$/gm).test(msg.toLowerCase())) { @@ -167,8 +256,14 @@ export class SSHService { }); conn .on("ready", async () => { + if (epoch !== this.epoch) { + // logged out (or torn down) while this handshake was in flight + conn.end(); + return reject(new Error("SSH connection superseded")); + } this.connectionPool.push(conn); this.connected = true; + this.emitState("connected"); if (!passwordBanner) { if (this.connectionPool.length === 1) { let test = await this.exec("ls"); @@ -189,9 +284,10 @@ export class SSHService { password: connectionInfo.password || undefined, privateKey: connectionInfo.privateKey || undefined, passphrase: connectionInfo.passphrase || undefined, - keepaliveInterval: 30000, + keepaliveInterval: SSHService.KEEPALIVE_INTERVAL_MS, + keepaliveCountMax: SSHService.KEEPALIVE_COUNT_MAX, tryKeyboard: true, - readyTimeout: 20000, + readyTimeout: SSHService.READY_TIMEOUT_MS, }); }); } @@ -201,9 +297,13 @@ export class SSHService { } async disconnect(reconnecting = false) { - log.info("DISCONNECT: connectionInfo", this.connectionInfo.host); this.loggingOut = true; + this.epoch++; + this.reconnectAbort?.abort(); // else backoff races the teardown and reopens what we're closing + this.reconnecting = false; try { + // connectionInfo is null after cancelVerification(); an unguarded deref here threw + log.info("DISCONNECT: connectionInfo", this.connectionInfo?.host ?? ""); this.connected = false; if (!reconnecting) { this.connectionInfo = null; @@ -233,6 +333,9 @@ export class SSHService { return error; } finally { this.loggingOut = false; + // Stay silent: an intentional teardown must not pop the reconnect modal on logout. + // Clearing lastState lets the next connect() emit "connected" again. + this.lastState = null; } } @@ -243,7 +346,7 @@ export class SSHService { async execCommand(command) { if (this.loggingOut) return { rc: -1, stdout: "", stderr: "Logging Out!" }; - const conn = this.getConnectionFromPool(); + const conn = await this.acquireConnection(); // an empty pool would otherwise surface as a TypeError on conn.exec if (!conn) return { rc: -1, stdout: "", stderr: "Not connected!" }; return new Promise((resolve, reject) => { @@ -530,7 +633,8 @@ export class SSHService { * @returns sftp session object */ async getSFTPSession(conn = null) { - conn = this.getConnectionFromPool(); + conn = conn ?? (await this.acquireConnection()); + if (!conn) throw new Error("SSH not connected, can't open an SFTP session"); return new Promise((resolve, reject) => { conn.sftp((err, sftp) => { if (err) { @@ -614,7 +718,8 @@ export class SSHService { * @param {Client} [conn] * @returns `void` */ - async downloadFileSSH(remotePath, localPath, conn = this.getConnectionFromPool()) { + async downloadFileSSH(remotePath, localPath, conn = null) { + conn = conn ?? (await this.acquireConnection()); return new Promise((resolve, reject) => { conn.exec(`sudo cat ${StringUtils.escapeStringForShell(remotePath)}`, async (err, stream) => { try { @@ -642,7 +747,8 @@ export class SSHService { * @param {Client} [conn] * @returns `true` if download was successful, `false` otherwise */ - async downloadDirectorySSH(remotePath, localPath, conn = this.getConnectionFromPool()) { + async downloadDirectorySSH(remotePath, localPath, conn = null) { + conn = conn ?? (await this.acquireConnection()); try { if (!fs.existsSync(localPath)) { fs.mkdirSync(localPath, { recursive: true }); @@ -672,7 +778,8 @@ export class SSHService { * @param {Client} [conn] * @returns `void` */ - async uploadFileSSH(localPath, remotePath, conn = this.getConnectionFromPool()) { + async uploadFileSSH(localPath, remotePath, conn = null) { + conn = conn ?? (await this.acquireConnection()); return new Promise((resolve, reject) => { conn.exec(`sudo cat > ${StringUtils.escapeStringForShell(remotePath)}`, async (err, stream) => { try { @@ -698,7 +805,8 @@ export class SSHService { * @param {String} remotePath * @param {Client} [conn] */ - async ensureRemotePathExists(remotePath, conn = this.getConnectionFromPool()) { + async ensureRemotePathExists(remotePath, conn = null) { + conn = conn ?? (await this.acquireConnection()); return new Promise((resolve, reject) => { conn.exec(`sudo mkdir -p ${remotePath} && sudo chown ${this.connectionInfo.user} ${remotePath}`, (err) => { if (err) reject(err); @@ -717,7 +825,7 @@ export class SSHService { async uploadDirectorySSH(localPath, remotePath, conn = null) { try { if (!conn) { - conn = await this.getConnectionFromPool(); + conn = await this.acquireConnection(); } await this.ensureRemotePathExists(remotePath); diff --git a/launcher/src/backend/tests/unit/SSHReconnect.test.js b/launcher/src/backend/tests/unit/SSHReconnect.test.js new file mode 100644 index 0000000000..5d0318a48a --- /dev/null +++ b/launcher/src/backend/tests/unit/SSHReconnect.test.js @@ -0,0 +1,195 @@ +// A 100ms setInterval used to grow the pool while it held fewer than six connections. In a +// blackhole nothing reached "ready", so it dialled ~10x/s until sshd's MaxStartups and fail2ban +// refused the client at the TCP layer - the next login was rejected instantly, not timed out. + +const mockCtl = { built: [], readyMode: "ready" }; + +jest.mock("ssh2", () => { + const { EventEmitter } = require("events"); + class FakeClient extends EventEmitter { + constructor() { + super(); + this._chanMgr = { _count: 0 }; + this.ended = false; + global.__sshMock.built.push(this); + } + connect() { + if (global.__sshMock.readyMode === "ready") { + setTimeout(() => this.emit("ready"), 0); + return; + } + if (global.__sshMock.readyMode === "manual") return; // the test drives "ready" by hand + // blackhole: nothing answers, ssh2 only gives up on readyTimeout (shortened here) + setTimeout(() => this.emit("error", new Error("Timed out while waiting for handshake")), 10); + } + exec(cmd, cb) { + const { EventEmitter: EE } = require("events"); + const stream = new EE(); + stream.stderr = new EE(); + cb(null, stream); + setTimeout(() => stream.emit("close", 0), 0); + } + end() { + this.ended = true; + this.emit("close"); + } + } + return { Client: FakeClient, utils: { generateKeyPairSync: () => ({}) } }; +}); + +global.__sshMock = mockCtl; + +const { SSHService } = require("../../SSHService.js"); + +const INFO = { host: "10.0.0.1", port: 22, user: "root", password: "x" }; +const REAL_DELAYS = SSHService.RECONNECT_DELAYS_MS; + +const settle = (ms = 20) => new Promise((r) => setTimeout(r, ms)); + +beforeEach(() => { + mockCtl.built = []; + mockCtl.readyMode = "ready"; + SSHService.RECONNECT_DELAYS_MS = [20, 20, 20]; +}); + +afterEach(() => { + SSHService.RECONNECT_DELAYS_MS = REAL_DELAYS; +}); + +test("constructing the service starts no background connection timer", async () => { + const ssh = new SSHService(); + ssh.connectionInfo = INFO; + + await settle(300); // the old poller would have opened ~30 connections + + expect(mockCtl.built).toHaveLength(0); +}); + +test("a blackholed network does not produce a connection storm", async () => { + const ssh = new SSHService(); + mockCtl.readyMode = "blackhole"; + ssh.connectionInfo = INFO; + + const attempts = [ssh.acquireConnection(), ssh.acquireConnection(), ssh.acquireConnection()]; + await settle(300); + + // single-flight: three callers share one dial, and no timer adds more + expect(mockCtl.built).toHaveLength(1); + void attempts; +}); + +test("a dead transport leaves the pool and the service stops reporting connected", async () => { + const ssh = new SSHService(); + await ssh.connect(INFO); + expect(ssh.connectionPool).toHaveLength(1); + expect(ssh.connected).toBe(true); + + mockCtl.readyMode = "blackhole"; // no reconnect should succeed + ssh.connectionPool[0].emit("close"); + + expect(ssh.connectionPool).toHaveLength(0); + expect(ssh.connected).toBe(false); + ssh.reconnectAbort?.abort(); +}); + +test("losing an established connection retries on the backoff schedule, one at a time", async () => { + const ssh = new SSHService(); + await ssh.connect(INFO); + const first = ssh.connectionPool[0]; + mockCtl.built = []; + + mockCtl.readyMode = "blackhole"; + first.emit("close"); + await settle(200); // spans all three stubbed 20ms delays + + // bounded by the schedule, never the old ~10/s + expect(mockCtl.built.length).toBeLessThanOrEqual(SSHService.RECONNECT_DELAYS_MS.length); + expect(ssh.lastState).toBe("disconnected"); +}); + +test("a successful backoff attempt restores the connection", async () => { + const ssh = new SSHService(); + await ssh.connect(INFO); + const first = ssh.connectionPool[0]; + + mockCtl.readyMode = "blackhole"; + first.emit("close"); + expect(ssh.connectionPool).toHaveLength(0); + + mockCtl.readyMode = "ready"; // the VPN comes back + await settle(200); + + expect(ssh.connectionPool.length).toBeGreaterThan(0); + expect(ssh.connected).toBe(true); + expect(ssh.lastState).toBe("connected"); +}); + +test("disconnect() does not throw when verification was cancelled", async () => { + const ssh = new SSHService(); + await ssh.connect(INFO); + + // cancelVerification() nulls connectionInfo but leaves the pool; disconnect() used to + // deref connectionInfo.host before its try block and throw - an instant login rejection + ssh.cancelVerification(); + expect(ssh.connectionInfo).toBeNull(); + + await expect(ssh.disconnect(true)).resolves.toBe(true); +}); + +test("an intentional disconnect does not trigger a reconnect", async () => { + const ssh = new SSHService(); + await ssh.connect(INFO); + mockCtl.built = []; + + await ssh.disconnect(); + await settle(200); + + expect(mockCtl.built).toHaveLength(0); + expect(ssh.reconnecting).toBe(false); +}); + +test("an intentional disconnect emits no state that would pop the reconnect modal", async () => { + const seen = []; + const ssh = new SSHService((state) => seen.push(state)); + await ssh.connect(INFO); + expect(seen).toEqual(["connected"]); + + await ssh.disconnect(); + await settle(50); + + expect(seen).toEqual(["connected"]); // logout stays silent + expect(ssh.lastState).toBeNull(); // ...but a later connect can announce itself again +}); + +test("logging out mid-reconnect discards a handshake that lands afterwards", async () => { + const ssh = new SSHService(); + await ssh.connect(INFO); + mockCtl.built = []; + + mockCtl.readyMode = "manual"; // the backoff's handshake will hang + ssh.connectionPool[0].emit("close"); + await settle(60); // backoff fires and starts a handshake + + const inFlight = mockCtl.built[mockCtl.built.length - 1]; + expect(inFlight).toBeDefined(); + + await ssh.disconnect(); // user clicks Logout on the reconnect modal + expect(ssh.connectionPool).toHaveLength(0); + + inFlight.emit("ready"); // the hung handshake completes after logout + await settle(30); + + // It must not resurrect the session: the main process outlives the renderer reload, + // so a pooled connection here would stay open against a node the user logged out of. + expect(ssh.connectionPool).toHaveLength(0); + expect(ssh.connected).toBe(false); +}); + +test("concurrent execs share a single new connection instead of opening one each", async () => { + const ssh = new SSHService(); + ssh.connectionInfo = INFO; + + await Promise.all([ssh.execCommand("ls"), ssh.execCommand("ls"), ssh.execCommand("ls")]); + + expect(mockCtl.built).toHaveLength(1); +}); diff --git a/launcher/src/background.js b/launcher/src/background.js index 8f5b05ff70..c508aff549 100755 --- a/launcher/src/background.js +++ b/launcher/src/background.js @@ -34,6 +34,10 @@ configManager.setServiceManager(serviceManager); const authenticationService = new AuthenticationService(nodeConnection); const tekuGasLimitConfig = new TekuGasLimitConfig(nodeConnection); const sshService = new SSHService(); +// Push transport state to the renderer so the UI can show reconnecting and clear itself again +nodeConnection.sshService.onStateChange = (state) => { + mainWindow?.webContents?.send("sshConnectionState", state); +}; const { globalShortcut } = require("electron"); const log = require("electron-log"); const stereumUpdater = new StereumUpdater(log, createWindow, isDevelopment); @@ -102,16 +106,10 @@ ipcMain.handle("reconnect", async () => { } }); +// Polled every 2s by the header. It used to open a throwaway SSH connection each time, which +// during an outage piled up 18s handshakes until fail2ban banned the client. The pool now keeps +// `connected` accurate via dropConnection, so this is just a read. ipcMain.handle("checkConnection", async () => { - await nodeConnection.sshService - .checkSSHConnection(nodeConnection.nodeConnectionParams, 18000) - .then((isConnected) => { - nodeConnection.sshService.connected = isConnected; - }) - .catch((error) => { - console.error("Error checking SSH connection:", error); - nodeConnection.sshService.connected = false; - }); return nodeConnection.sshService.connected; }); diff --git a/launcher/src/components/UI/base-header/components/modals/ReconnectModal.vue b/launcher/src/components/UI/base-header/components/modals/ReconnectModal.vue index fc1873c52b..7315324421 100644 --- a/launcher/src/components/UI/base-header/components/modals/ReconnectModal.vue +++ b/launcher/src/components/UI/base-header/components/modals/ReconnectModal.vue @@ -40,10 +40,12 @@