diff --git a/src/main/runtime/rpc/methods/terminal.ts b/src/main/runtime/rpc/methods/terminal.ts index 7f12d75b0..48b1bc793 100644 --- a/src/main/runtime/rpc/methods/terminal.ts +++ b/src/main/runtime/rpc/methods/terminal.ts @@ -94,6 +94,12 @@ type TerminalMultiplexStream = { unsubscribeFit: () => void unsubscribeDriver: () => void unregisterBinaryHandler: () => void + // Why: the exit-wait promise for this slot is only removed from the runtime's + // waiter set on real PTY exit. Aborting this on detach releases it on slot + // unsubscribe, tab-switch re-subscribe, and connection close instead of + // leaking a waiter (and the closed-connection handler context it captures) + // for the life of a never-exiting agent terminal. + exitWaiterAbort: AbortController } type TerminalOutputChunk = { @@ -1256,6 +1262,9 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ stream.unsubscribeDriver() stream.unregisterBinaryHandler() streams.delete(streamId) + // Why: release the runtime exit-waiter for this slot (see the field's + // note). The .catch below no-ops because the stream is already deleted. + stream.exitWaiterAbort.abort() if (stream.isMobile && stream.client?.id) { runtime.handleMobileUnsubscribe(stream.ptyId, stream.client.id) } @@ -1472,7 +1481,8 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ unsubscribeResize: () => {}, unsubscribeFit: () => {}, unsubscribeDriver: () => {}, - unregisterBinaryHandler: () => {} + unregisterBinaryHandler: () => {}, + exitWaiterAbort: new AbortController() } streams.set(request.streamId, stream) stream.unregisterBinaryHandler = registerBinaryStreamHandler(request.streamId, (frame) => @@ -1660,7 +1670,10 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ sendResizedFrame(stream, event) }) void runtime - .waitForTerminal(request.terminal, { condition: 'exit' }) + .waitForTerminal(request.terminal, { + condition: 'exit', + signal: stream.exitWaiterAbort.signal + }) .then(() => { if (streams.get(request.streamId) === stream) { detachStream(request.streamId, true) @@ -1797,8 +1810,10 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ }, connectionId ) + // Why: bind the exit-waiter to the connection dispatch signal so it is + // removed on socket close/error instead of leaking until real exit. void runtime - .waitForTerminal(params.terminal, { condition: 'exit' }) + .waitForTerminal(params.terminal, { condition: 'exit', signal }) .then(() => runtime.cleanupSubscription(subscriptionId)) .catch(() => runtime.cleanupSubscription(subscriptionId)) }) @@ -1847,8 +1862,10 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ }, connectionId ) + // Why: bind the exit-waiter to the connection dispatch signal so it is + // removed on socket close/error instead of leaking until real exit. void runtime - .waitForTerminal(params.terminal, { condition: 'exit' }) + .waitForTerminal(params.terminal, { condition: 'exit', signal }) .then(() => runtime.cleanupSubscription(subscriptionId)) .catch(() => runtime.cleanupSubscription(subscriptionId)) const sendFrame = ( diff --git a/src/main/runtime/terminal-subscribe-exit-waiter-leak.test.ts b/src/main/runtime/terminal-subscribe-exit-waiter-leak.test.ts new file mode 100644 index 000000000..c981ad96e --- /dev/null +++ b/src/main/runtime/terminal-subscribe-exit-waiter-leak.test.ts @@ -0,0 +1,88 @@ +/** + * Memory-leak regression: terminal.subscribe / terminal.multiplex register a + * runtime exit-waiter (waitForTerminal condition:'exit') per subscribed slot. + * Without an AbortSignal that waiter is only removed on real PTY exit, so for a + * never-exiting agent terminal every remote/mobile reconnect and tab-switch + * re-subscribe leaked a waiter (and the closed-connection handler context it + * captures). The subscribe paths now pass a signal — this pins that a signalled + * exit-waiter is released when the signal aborts, and an unsignalled one is not. + */ +import { describe, expect, it } from 'vitest' +import { OrcaRuntimeService } from './orca-runtime' +import type { RuntimeTerminalWait } from '../../shared/runtime-types' + +type RuntimeInternals = { + recordPtyWorktree: (ptyId: string, worktreeId: string, state?: { connected?: boolean }) => unknown + handleByPtyId: Map + waitersByHandle: Map> +} + +function internals(runtime: OrcaRuntimeService): RuntimeInternals { + return runtime as unknown as RuntimeInternals +} + +// Register a live, connected PTY that a handle resolves to, so waitForTerminal +// with condition:'exit' registers a pending waiter instead of resolving early. +function registerLivePty(runtime: OrcaRuntimeService, ptyId: string, handle: string): void { + internals(runtime).recordPtyWorktree(ptyId, 'wt-live', { connected: true }) + internals(runtime).handleByPtyId.set(ptyId, handle) +} + +function waiterCount(runtime: OrcaRuntimeService, handle: string): number { + return internals(runtime).waitersByHandle.get(handle)?.size ?? 0 +} + +describe('terminal.subscribe exit-waiter leak regression', () => { + it('releases a signalled exit-waiter when the signal aborts', async () => { + const runtime = new OrcaRuntimeService() + registerLivePty(runtime, 'pty-live', 'handle-live') + + const controller = new AbortController() + const wait = runtime.waitForTerminal('handle-live', { + condition: 'exit', + signal: controller.signal + }) + // Swallow the abort rejection; we assert on the waiter set, not the result. + const settled: Promise = wait.catch(() => 'aborted' as const) + await Promise.resolve() + + expect(waiterCount(runtime, 'handle-live')).toBe(1) + + // Simulate detachStream / connection close aborting the slot's controller. + controller.abort() + await expect(settled).resolves.toBe('aborted') + + expect(waiterCount(runtime, 'handle-live')).toBe(0) + }) + + it('leaks an unsignalled exit-waiter across reconnects (documents the bug the fix prevents)', async () => { + const runtime = new OrcaRuntimeService() + registerLivePty(runtime, 'pty-live', 'handle-live') + + // Three subscribes with no signal, as the pre-fix subscribe paths did. + for (let i = 0; i < 3; i += 1) { + void runtime.waitForTerminal('handle-live', { condition: 'exit' }).catch(() => {}) + } + await Promise.resolve() + + // Nothing frees them short of real PTY exit — they accumulate. + expect(waiterCount(runtime, 'handle-live')).toBe(3) + }) + + it('does not leak when many signalled subscribes churn (reconnect simulation)', async () => { + const runtime = new OrcaRuntimeService() + registerLivePty(runtime, 'pty-live', 'handle-live') + + for (let i = 0; i < 25; i += 1) { + const controller = new AbortController() + const settled = runtime + .waitForTerminal('handle-live', { condition: 'exit', signal: controller.signal }) + .catch(() => 'aborted' as const) + await Promise.resolve() + controller.abort() + await settled + } + + expect(waiterCount(runtime, 'handle-live')).toBe(0) + }) +})