From f6631ddeee014d91445fef484b0e82509097d8fe Mon Sep 17 00:00:00 2001 From: OrcaWin Date: Sat, 26 Sep 2026 13:55:22 -0700 Subject: [PATCH] fix(daemon): release terminal attach cancellation listeners (#23191) * fix(daemon): release terminal attach cancellation listeners * fix(daemon): preserve attach wait settlement ordering * fix(i18n): register existing diff note fallback strings --------- Co-authored-by: m4air --- .../terminal-attach-cancellation.test.ts | 38 +++++++++++++++++++ .../daemon/terminal-attach-cancellation.ts | 22 +++++------ .../daemon/terminal-host-session-create.ts | 9 +++-- src/main/daemon/terminal-host.ts | 4 +- 4 files changed, 56 insertions(+), 17 deletions(-) create mode 100644 src/main/daemon/terminal-attach-cancellation.test.ts diff --git a/src/main/daemon/terminal-attach-cancellation.test.ts b/src/main/daemon/terminal-attach-cancellation.test.ts new file mode 100644 index 00000000000..1c9fa0a1ea3 --- /dev/null +++ b/src/main/daemon/terminal-attach-cancellation.test.ts @@ -0,0 +1,38 @@ +import { describe, expect, it, vi } from 'vitest' +import { waitForTerminalAttachOperation } from './terminal-attach-cancellation' + +describe('terminal attach cancellation', () => { + it('removes cancellation listeners when the operation settles first', async () => { + const controller = new AbortController() + const removeListener = vi.spyOn(controller.signal, 'removeEventListener') + + await expect( + waitForTerminalAttachOperation(Promise.resolve('ready'), controller.signal, 'session-1') + ).resolves.toBe('ready') + + expect(removeListener).toHaveBeenCalledTimes(1) + }) + + it('preserves operation-first ordering when settlement and abort share a turn', async () => { + const controller = new AbortController() + const operation = Promise.withResolvers() + const waiting = waitForTerminalAttachOperation( + operation.promise, + controller.signal, + 'session-3' + ) + operation.resolve('ready') + controller.abort() + await expect(waiting).resolves.toBe('ready') + }) + + it('rejects promptly on cancellation while the operation remains pending', async () => { + const controller = new AbortController() + const operation = new Promise(() => {}) + const waiting = waitForTerminalAttachOperation(operation, controller.signal, 'session-2') + + controller.abort() + + await expect(waiting).rejects.toMatchObject({ name: 'TerminalAttachCanceledError' }) + }) +}) diff --git a/src/main/daemon/terminal-attach-cancellation.ts b/src/main/daemon/terminal-attach-cancellation.ts index 9e87cbdb1bc..21c037d91d3 100644 --- a/src/main/daemon/terminal-attach-cancellation.ts +++ b/src/main/daemon/terminal-attach-cancellation.ts @@ -1,17 +1,17 @@ import { TerminalAttachCanceledError } from './daemon-errors' +import { PromiseSettlementWaiters } from '../../shared/promise-settlement-waiters' -/** Never resolves; only rejects, so it can bound a wait without settling it. */ -export function rejectOnAbort(signal: AbortSignal | undefined, sessionId: string): Promise { +export function waitForTerminalAttachOperation( + operation: Promise, + signal: AbortSignal | undefined, + sessionId: string +): Promise { if (!signal) { - return new Promise(() => {}) + return operation } - return new Promise((_resolve, reject) => { - if (signal.aborted) { - reject(new TerminalAttachCanceledError(sessionId)) - return - } - signal.addEventListener('abort', () => reject(new TerminalAttachCanceledError(sessionId)), { - once: true - }) + return new PromiseSettlementWaiters(operation).wait({ + signal, + abortInMicrotask: true, + createAbortError: () => new TerminalAttachCanceledError(sessionId) }) } diff --git a/src/main/daemon/terminal-host-session-create.ts b/src/main/daemon/terminal-host-session-create.ts index 7a8dbe379cc..15e86a70a72 100644 --- a/src/main/daemon/terminal-host-session-create.ts +++ b/src/main/daemon/terminal-host-session-create.ts @@ -12,7 +12,7 @@ import type { TerminalHostTombstones } from './terminal-host-tombstones' import type { TerminalSessionTeardown } from './terminal-session-teardown' import { resolveDaemonSessionScrollbackRows } from './daemon-session-scrollback-window' import { TerminalAttachCanceledError } from './daemon-errors' -import { rejectOnAbort } from './terminal-attach-cancellation' +import { waitForTerminalAttachOperation } from './terminal-attach-cancellation' import { SessionNotFoundError } from './types' import { resolveWslSessionContext } from './wsl-session-context' @@ -47,10 +47,11 @@ export async function createOrAttachTerminalSession( // reaches this a beat after the attach that retired it, and refusing surfaced the raw // SessionNotFoundError to the user. Windows makes it the common case, where the plain-shell // sweep holds the claim across an OS identity probe and taskkill (#18046). - await Promise.race([ + await waitForTerminalAttachOperation( deps.sessionTeardown.settle(opts.sessionId), - rejectOnAbort(opts.cancelSignal, opts.sessionId) - ]) + opts.cancelSignal, + opts.sessionId + ) deps.assertCreateAllowed() existing = deps.sessions.get(opts.sessionId) // Unkillable child, or a fresh teardown claimed it while we waited: still nobody's to recreate. diff --git a/src/main/daemon/terminal-host.ts b/src/main/daemon/terminal-host.ts index 8ccfe31b707..6e8d5ceee81 100644 --- a/src/main/daemon/terminal-host.ts +++ b/src/main/daemon/terminal-host.ts @@ -21,7 +21,7 @@ import { TerminalHostTombstones } from './terminal-host-tombstones' import { listLiveTerminalHostSessions } from './terminal-host-session-listing' import { createOrAttachTerminalSession } from './terminal-host-session-create' import { TerminalAttachCanceledError } from './daemon-errors' -import { rejectOnAbort } from './terminal-attach-cancellation' +import { waitForTerminalAttachOperation } from './terminal-attach-cancellation' import { randomUUID } from 'node:crypto' import { pruneRetiredPtyIncarnations } from '../../shared/retired-pty-incarnations' import { @@ -84,7 +84,7 @@ export class TerminalHost { // Why: the create ahead of us can be stuck on an unreachable share for // minutes. Waiting unconditionally is what let one dead path strand every // later create and attach for the session, so a canceled caller leaves. - await Promise.race([inFlight, rejectOnAbort(opts.cancelSignal, opts.sessionId)]) + await waitForTerminalAttachOperation(inFlight, opts.cancelSignal, opts.sessionId) this.assertCreateOrAttachAllowed(opts) } this.assertCreateOrAttachAllowed(opts)