From 39ef1c1f3dabc7a1b32154ff4dab381a34e0c860 Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Sun, 13 Sep 2026 20:57:24 +0530 Subject: [PATCH] refactor(desktop): keepalive redials with capped backoff; staleness derived, not counted A dead tunnel was redialled every 2 s until the scope was torn down; delays now double from the base to a 30 s cap and reset on open. The generation counter duplicated what the entry map and entry.socket already say (stop() removes the entry, connect() replaces the socket), so abandonIfStale reads those instead. afterStop's unused entry parameter is dropped. --- apps/desktop/electron/pool-stop.ts | 4 +-- .../electron/ssh-isolated-keepalive.test.ts | 17 +++++++++-- .../electron/ssh-isolated-keepalive.ts | 30 +++++++++++-------- 3 files changed, 34 insertions(+), 17 deletions(-) diff --git a/apps/desktop/electron/pool-stop.ts b/apps/desktop/electron/pool-stop.ts index 80f994f501..7215d11c1a 100644 --- a/apps/desktop/electron/pool-stop.ts +++ b/apps/desktop/electron/pool-stop.ts @@ -37,7 +37,7 @@ export interface PoolStopperDeps { * Extra per-key work that must finish before a replacement may spawn. * Held on the same in-flight promise as child exit (SSH teardown, etc.). */ - afterStop?: (key: string, entry: PoolStopEntry) => Promise + afterStop?: (key: string) => Promise } export interface PoolStopper { @@ -75,7 +75,7 @@ export function createPoolStopper(deps: PoolStopperDeps): PoolStopper { deps.stopChild(entry.process) await deps.waitForExit(entry.process) if (deps.afterStop) { - await deps.afterStop(key, entry) + await deps.afterStop(key) } })().finally(() => { stops.delete(key) diff --git a/apps/desktop/electron/ssh-isolated-keepalive.test.ts b/apps/desktop/electron/ssh-isolated-keepalive.test.ts index 0b21331941..dd2aa9a3e9 100644 --- a/apps/desktop/electron/ssh-isolated-keepalive.test.ts +++ b/apps/desktop/electron/ssh-isolated-keepalive.test.ts @@ -66,16 +66,29 @@ describe('ssh-isolated keep-alive registry (#106935)', () => { instances[0].emit('open') expect(registry.openUrl('conn:office::work')).toBe('ws://127.0.0.1:53101/api/ws?token=sess-work') + // A dropped socket is redialled with backoff (25 → 50 ms), so a dead tunnel is not + // hammered every interval until the scope is torn down. + vi.useFakeTimers() + instances[1].emit('close', { code: 1006 }) + await vi.advanceTimersByTimeAsync(25) + expect(instances).toHaveLength(3) + instances[2].emit('close', { code: 1006 }) + await vi.advanceTimersByTimeAsync(25) + expect(instances).toHaveLength(3) + await vi.advanceTimersByTimeAsync(25) + expect(instances).toHaveLength(4) + vi.useRealTimers() + registry.stop('conn:office::work') expect(instances[0].closed).toBe(true) expect(registry.isArmed('conn:office::work')).toBe(false) // Tearing down one sibling must not drop the other owned scope. - expect(instances[1].closed).toBe(false) + expect(instances[3].closed).toBe(false) expect(registry.isArmed('conn:office::less')).toBe(true) instances[0].emit('close', { code: 1006 }) await new Promise(resolve => setTimeout(resolve, 50)) - expect(instances).toHaveLength(2) + expect(instances).toHaveLength(4) expect(registry.openUrl('conn:office::work')).toBeNull() }) diff --git a/apps/desktop/electron/ssh-isolated-keepalive.ts b/apps/desktop/electron/ssh-isolated-keepalive.ts index 43b8f61f04..59677db4fc 100644 --- a/apps/desktop/electron/ssh-isolated-keepalive.ts +++ b/apps/desktop/electron/ssh-isolated-keepalive.ts @@ -8,8 +8,8 @@ * `sshConnections`. Sticky spawn artifacts (owner-nonce, token file, lockfile) * are NOT liveness and must not suppress idle-exit. * - * If the socket drops the registry reconnects after a delay; while it is down - * the backend may idle-exit as before. This module never consults + * If the socket drops the registry reconnects with capped exponential backoff; + * while it is down the backend may idle-exit as before. This module never consults * nonce/lock/token files. */ import { buildGatewayWsUrl } from './connection-config' @@ -27,7 +27,7 @@ export type SshIsolatedKeepaliveOptions = { } type KeepaliveEntry = { - generation: number + failures: number reconnectTimer: ReturnType | null scope: string socket: { close?: () => void; url?: string } | null @@ -35,6 +35,7 @@ type KeepaliveEntry = { } const DEFAULT_RECONNECT_DELAY_MS = 2_000 +const MAX_RECONNECT_DELAY_MS = 30_000 function addListener(socket: any, type: string, handler: (event?: any) => void) { if (typeof socket?.addEventListener === 'function') { @@ -84,10 +85,13 @@ export function createSshIsolatedKeepaliveRegistry(options: SshIsolatedKeepalive return } + // A dead tunnel would otherwise be redialled every 2 s until the scope is torn down. + const delay = Math.min(reconnectDelayMs * 2 ** entry.failures, MAX_RECONNECT_DELAY_MS) + entry.failures += 1 entry.reconnectTimer = setTimeout(() => { entry.reconnectTimer = null connect(entry) - }, reconnectDelayMs) + }, delay) } function connect(entry: KeepaliveEntry) { @@ -95,8 +99,6 @@ export function createSshIsolatedKeepaliveRegistry(options: SshIsolatedKeepalive return } - entry.generation += 1 - const generation = entry.generation clearTimer(entry) closeSocket(entry) @@ -114,18 +116,21 @@ export function createSshIsolatedKeepaliveRegistry(options: SshIsolatedKeepalive entry.socket = socket + // Staleness is derivable: stop() removes the entry, connect() replaces entry.socket. const abandonIfStale = () => { - if (entries.get(entry.scope) !== entry || entry.generation !== generation) { + if (entries.get(entry.scope) !== entry || entry.socket !== socket) { return } - if (entry.socket === socket) { - entry.socket = null - } - + entry.socket = null scheduleReconnect(entry) } + addListener(socket, 'open', () => { + if (entry.socket === socket) { + entry.failures = 0 + } + }) addListener(socket, 'close', abandonIfStale) addListener(socket, 'error', abandonIfStale) } @@ -146,7 +151,7 @@ export function createSshIsolatedKeepaliveRegistry(options: SshIsolatedKeepalive stop(scope) const entry: KeepaliveEntry = { - generation: 0, + failures: 0, reconnectTimer: null, scope, socket: null, @@ -164,7 +169,6 @@ export function createSshIsolatedKeepaliveRegistry(options: SshIsolatedKeepalive } entries.delete(scope) - entry.generation += 1 clearTimer(entry) closeSocket(entry) }