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.
This commit is contained in:
kshitijk4poor
2026-09-13 20:57:24 +05:30
committed by kshitij
parent e24b344b6c
commit 39ef1c1f3d
3 changed files with 34 additions and 17 deletions
+2 -2
View File
@@ -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<void>
afterStop?: (key: string) => Promise<void>
}
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)
@@ -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()
})
+17 -13
View File
@@ -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<typeof setTimeout> | 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)
}