diff --git a/src/main/ssh/ssh-relay-live-daemon-harness.ts b/src/main/ssh/ssh-relay-live-daemon-harness.ts index dea87d8e2a1..b8e8df6da43 100644 --- a/src/main/ssh/ssh-relay-live-daemon-harness.ts +++ b/src/main/ssh/ssh-relay-live-daemon-harness.ts @@ -3,7 +3,7 @@ // // Needs `pnpm build:relay` — callers skip when relayBundleDirForHost() is null. -import { spawn, type ChildProcess } from 'node:child_process' +import { spawn, spawnSync, type ChildProcess } from 'node:child_process' import { randomBytes } from 'node:crypto' import { EventEmitter } from 'node:events' import { cpSync, existsSync, mkdirSync, symlinkSync, writeFileSync } from 'node:fs' @@ -32,6 +32,51 @@ export function isProcessAlive(pid: number): boolean { } } +/** + * SIGKILL a process and every descendant, deepest first. + * + * Why: a relay killed on its own leaves its PTY shells reparented to init — the very + * leak these tests are about, reproduced on the machine running them. + */ +export function killProcessTree(pid: number): void { + const listing = spawnSync('ps', ['-eo', 'pid=,ppid=']).stdout?.toString() ?? '' + const childrenByParent = new Map() + for (const line of listing.split('\n')) { + const [child, parent] = line.trim().split(/\s+/).map(Number) + if (Number.isInteger(child) && Number.isInteger(parent)) { + childrenByParent.set(parent, [...(childrenByParent.get(parent) ?? []), child]) + } + } + const ordered: number[] = [] + const visit = (current: number): void => { + for (const child of childrenByParent.get(current) ?? []) { + visit(child) + } + ordered.push(current) + } + visit(pid) + for (const target of ordered) { + try { + process.kill(target, 'SIGKILL') + } catch { + /* already gone */ + } + } +} + +export function waitForExit(child: ChildProcess, timeoutMs = 20_000): Promise { + if (child.exitCode !== null) { + return Promise.resolve(child.exitCode) + } + return new Promise((resolve, reject) => { + const timer = setTimeout(() => reject(new Error('relay daemon never exited')), timeoutMs) + child.once('exit', (code) => { + clearTimeout(timer) + resolve(code) + }) + }) +} + export class LiveRelayFixture { readonly sockPath: string readonly credentialFile: string @@ -112,7 +157,8 @@ export class LiveRelayFixture { return bridge } - dispose(): void { + /** Ask each daemon to shut down (which disposes its PTYs), then kill whatever is left. */ + async dispose(): Promise { for (const child of this.children) { try { child.kill('SIGKILL') @@ -122,11 +168,17 @@ export class LiveRelayFixture { } for (const pid of this.daemonPids) { try { - process.kill(pid, 'SIGKILL') + process.kill(pid, 'SIGTERM') } catch { /* already gone */ } } + for (let attempt = 0; attempt < 30 && [...this.daemonPids].some(isProcessAlive); attempt++) { + await delay(100) + } + for (const pid of this.daemonPids) { + killProcessTree(pid) + } } } @@ -160,13 +212,16 @@ export class RelayBridge { private ingest(chunk: Buffer): void { this.buffer = Buffer.concat([this.buffer, chunk]) - if (!this.sentinelSeen) { + while (!this.sentinelSeen) { const newline = this.buffer.indexOf('\n') if (newline === -1) { return } - this.sentinelSeen = true + // Why: consume whole lines until the sentinel; a stray pre-sentinel line would + // otherwise be mistaken for it and desynchronise every frame after it. + const line = this.buffer.subarray(0, newline).toString('utf-8') this.buffer = this.buffer.subarray(newline + 1) + this.sentinelSeen = line.includes('ORCA-RELAY') } while (this.buffer.length >= FRAME_HEADER_BYTES) { const length = this.buffer.readUInt32BE(9) diff --git a/src/main/ssh/ssh-relay-socket-owner.integration.test.ts b/src/main/ssh/ssh-relay-socket-owner.integration.test.ts index 3e518efe3d1..eef231ca3ca 100644 --- a/src/main/ssh/ssh-relay-socket-owner.integration.test.ts +++ b/src/main/ssh/ssh-relay-socket-owner.integration.test.ts @@ -21,7 +21,9 @@ import { createLocalShellConnection, delay, isProcessAlive, - relayBundleDirForHost + killProcessTree, + relayBundleDirForHost, + waitForExit } from './ssh-relay-live-daemon-harness' import { isRelaySocketOwnerLiveError, @@ -42,8 +44,8 @@ describe.skipIf(!BUNDLE_DIR || process.platform === 'win32')('relay socket owner let fixture: LiveRelayFixture | null = null let root: string | null = null - afterEach(() => { - fixture?.dispose() + afterEach(async () => { + await fixture?.dispose() fixture = null if (root) { rmSync(root, { recursive: true, force: true }) @@ -77,7 +79,7 @@ describe.skipIf(!BUNDLE_DIR || process.platform === 'win32')('relay socket owner await expect(probeRelaySocketOwner(conn, process.execPath, live.sockPath)).resolves.toBe('live') // A crashed relay leaves the inode behind; that, and only that, is a stale socket. - process.kill(pid, 'SIGKILL') + killProcessTree(pid) await delay(500) expect(existsSync(live.sockPath)).toBe(true) await expect(probeRelaySocketOwner(conn, process.execPath, live.sockPath)).resolves.toBe( @@ -98,8 +100,8 @@ describe.skipIf(!BUNDLE_DIR || process.platform === 'win32')('relay socket owner // The socket kept its owner, so a fresh daemon hits EADDRINUSE, finds a live // listener, and refuses to start — the interlock the unconditional rm -f defeated. - live.launchDaemon('replacement') - await delay(2_000) + const replacement = live.launchDaemon('replacement') + expect(await waitForExit(replacement)).not.toBe(0) const bridge = live.openBridge() await bridge.waitForSentinel() const status = await bridge.request('relay.status')