mirror of
https://github.com/stablyai/orca.git
synced 2026-10-22 16:03:47 +00:00
test(ssh): stop the live-relay harness from leaking PTY children
Killing a relay on its own reparents its PTY shells to init — the leak these tests are about, reproduced on the machine running them. Terminate the daemon first so it disposes its PTYs, then kill whatever survives, deepest first. Also waits for the replacement daemon to actually exit instead of sleeping, and consumes whole lines until the relay sentinel rather than assuming the first one.
This commit is contained in:
@@ -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<number, number[]>()
|
||||
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<number | null> {
|
||||
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<void> {
|
||||
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)
|
||||
|
||||
@@ -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<RelayStatus>('relay.status')
|
||||
|
||||
Reference in New Issue
Block a user