From ba85f1a6f3ccaf9cd43559dcd562b9a6c29191b2 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Mon, 26 Oct 2026 15:22:58 -0700 Subject: [PATCH] fix(main): share one window listener across overlapping requests to stop MaxListeners warnings (#33102) * fix(main): share one window/WebContents listener across overlapping requests Per-request lifecycle watchers (renderer-gone aborts, tab-close relays, project-move pause, SSH-access link, Jira summary, workspace-cleanup scans) each added their own destroyed / render-process-gone / did-navigate / closed listener to the long-lived main window. They were removed on settle, but with a dozen remote-runtime subscriptions or tab closes in flight they stacked on top of the window's permanent listeners and tripped MaxListenersExceededWarning. addFanoutListener keeps one native listener per (emitter, event) and fans out to a callback set, detaching when the last subscriber releases. * test(fs-watcher): await watcher conditions instead of sleeping * test(window): one lifecycle event rejects every overlapping close request --- src/main/event-listener-fanout.test.ts | 74 +++++++++++++++++ src/main/event-listener-fanout.ts | 82 +++++++++++++++++++ .../filesystem-watcher-local-events.test.ts | 7 +- ...lesystem-watcher-local-unsubscribe.test.ts | 26 +++--- .../filesystem-watcher-test-sender.ts | 8 +- src/main/ipc/jira/issue-summary-generation.ts | 5 +- .../native-chat-subscribe-lifecycle.test.ts | 4 +- src/main/ipc/renderer-lifetime-abort.test.ts | 15 ++++ src/main/ipc/renderer-lifetime-abort.ts | 33 +++----- ...ment-subscription-layout-recording.test.ts | 2 +- ...nvironments-subscription-lifecycle.test.ts | 12 +-- ...-environments-subscription-routing.test.ts | 10 +-- ...environments-subscription-teardown.test.ts | 12 +-- src/main/ipc/runtime-ssh-access-handlers.ts | 5 +- .../workspace-cleanup-snapshot-ipc.test.ts | 5 +- src/main/ipc/workspace-cleanup.ts | 7 +- src/main/media/media-preview-protocol.test.ts | 2 +- src/main/media/media-preview-protocol.ts | 2 +- .../dropped-path-preparation-ipc.test.ts | 4 +- src/main/window/project-move-pause.test.ts | 19 +++++ src/main/window/project-move-pause.ts | 20 ++--- .../session-tab-close-request-relay.test.ts | 60 ++++++++++++++ .../window/session-tab-close-request-relay.ts | 24 +++--- .../terminal-tab-close-request-relay.test.ts | 27 ++++++ .../terminal-tab-close-request-relay.ts | 25 +++--- 25 files changed, 381 insertions(+), 109 deletions(-) create mode 100644 src/main/event-listener-fanout.test.ts create mode 100644 src/main/event-listener-fanout.ts diff --git a/src/main/event-listener-fanout.test.ts b/src/main/event-listener-fanout.test.ts new file mode 100644 index 00000000000..8fc6717aab5 --- /dev/null +++ b/src/main/event-listener-fanout.test.ts @@ -0,0 +1,74 @@ +import { EventEmitter } from 'node:events' +import { describe, expect, it, vi } from 'vitest' +import { addFanoutListener, addFanoutListeners } from './event-listener-fanout' + +describe('addFanoutListener', () => { + it('shares one native listener across overlapping subscribers and returns to baseline', () => { + const source = new EventEmitter() + const callbacks = Array.from({ length: 25 }, () => vi.fn()) + const releases = callbacks.map((callback) => addFanoutListener(source, 'destroyed', callback)) + + expect(source.listenerCount('destroyed')).toBe(1) + source.emit('destroyed', 'arg') + for (const callback of callbacks) { + expect(callback).toHaveBeenCalledWith('arg') + } + + for (const release of releases.slice(1)) { + release() + } + expect(source.listenerCount('destroyed')).toBe(1) + releases[0]() + releases[0]() + expect(source.eventNames()).toEqual([]) + }) + + it('lets the same callback subscribe twice without one release dropping the other', () => { + const source = new EventEmitter() + const callback = vi.fn() + const releaseFirst = addFanoutListener(source, 'closed', callback) + addFanoutListener(source, 'closed', callback) + + releaseFirst() + source.emit('closed') + + expect(callback).toHaveBeenCalledTimes(1) + }) + + it('skips subscribers released by an earlier callback in the same dispatch', () => { + const source = new EventEmitter() + const second = vi.fn() + let releaseSecond = (): void => {} + addFanoutListener(source, 'destroyed', () => releaseSecond()) + releaseSecond = addFanoutListener(source, 'destroyed', second) + + source.emit('destroyed') + + expect(second).not.toHaveBeenCalled() + }) + + it('reattaches a fresh native listener after the last subscriber left', () => { + const source = new EventEmitter() + addFanoutListener(source, 'destroyed', vi.fn())() + const callback = vi.fn() + addFanoutListener(source, 'destroyed', callback) + + source.emit('destroyed') + + expect(source.listenerCount('destroyed')).toBe(1) + expect(callback).toHaveBeenCalledOnce() + }) +}) + +describe('addFanoutListeners', () => { + it('releases every event with one call', () => { + const source = new EventEmitter() + const release = addFanoutListeners(source, ['destroyed', 'render-process-gone'], vi.fn()) + + expect(source.eventNames().sort()).toEqual(['destroyed', 'render-process-gone']) + release() + release() + + expect(source.eventNames()).toEqual([]) + }) +}) diff --git a/src/main/event-listener-fanout.ts b/src/main/event-listener-fanout.ts new file mode 100644 index 00000000000..01f008e35bc --- /dev/null +++ b/src/main/event-listener-fanout.ts @@ -0,0 +1,82 @@ +/** The listener surface of Node's EventEmitter, which WebContents, BrowserWindow and ipcMain share. */ +export type FanoutEventSource = Pick + +// Callers declare the emitter's own argument types, exactly as with a direct `on`. +type FanoutCallback = (...args: any[]) => void + +type EventFanout = { + listener: FanoutCallback + callbacks: Set +} + +const fanoutsBySource = new WeakMap>() + +/** + * Subscribe through one shared native listener per (emitter, event). + * Why: per-request listeners on long-lived emitters (main window, its WebContents, ipcMain) + * pile up past Node's 10-listener warning whenever operations overlap, even with cleanup. + */ +export function addFanoutListener( + source: FanoutEventSource, + event: string, + callback: FanoutCallback +): () => void { + let fanouts = fanoutsBySource.get(source) + if (!fanouts) { + fanouts = new Map() + fanoutsBySource.set(source, fanouts) + } + let fanout = fanouts.get(event) + if (!fanout) { + const callbacks = new Set() + const created: EventFanout = { + callbacks, + // Snapshot so callbacks that unsubscribe or subscribe while dispatching stay well-defined. + listener: (...args) => { + for (const registered of Array.from(callbacks)) { + if (callbacks.has(registered)) { + registered(...args) + } + } + } + } + fanout = created + fanouts.set(event, created) + source.on(event, created.listener) + } + const active = fanout + const ownCallback: FanoutCallback = (...args) => callback(...args) + active.callbacks.add(ownCallback) + return () => { + if (!active.callbacks.delete(ownCallback) || active.callbacks.size > 0) { + return + } + source.removeListener(event, active.listener) + const current = fanoutsBySource.get(source) + if (current?.get(event) === active) { + current.delete(event) + if (current.size === 0) { + fanoutsBySource.delete(source) + } + } + } +} + +/** Subscribe one callback to several lifecycle events; the returned function releases all of them. */ +export function addFanoutListeners( + source: FanoutEventSource, + events: readonly string[], + callback: FanoutCallback +): () => void { + const releases = events.map((event) => addFanoutListener(source, event, callback)) + let released = false + return () => { + if (released) { + return + } + released = true + for (const release of releases) { + release() + } + } +} diff --git a/src/main/ipc/filesystem/watchers/filesystem-watcher-local-events.test.ts b/src/main/ipc/filesystem/watchers/filesystem-watcher-local-events.test.ts index 163b31c59c0..eb8da3a8bac 100644 --- a/src/main/ipc/filesystem/watchers/filesystem-watcher-local-events.test.ts +++ b/src/main/ipc/filesystem/watchers/filesystem-watcher-local-events.test.ts @@ -354,15 +354,16 @@ describe('local filesystem watcher flush serialization', () => { // Fake refresh revives cleared handles; this regression needs real Node Timeout behavior. vi.useRealTimers() statMock.mockResolvedValue({ isDirectory: () => true }) - const listener = { ...sender, id: 7, removeListener: vi.fn(), once: vi.fn() } + const listener = { ...sender, id: 7, removeListener: vi.fn(), on: vi.fn() } try { await subscribeLocalWatcher('/repo', listener as never) watcherCallback?.(null, [{ type: 'delete', path: '/repo/file.ts' }]) unsubscribeLocalWatcher('/repo', listener.id) await subscribeLocalWatcher('/repo', listener as never) watcherCallback?.(null, [{ type: 'delete', path: '/repo/file.ts' }]) - await new Promise((resolve) => setTimeout(resolve, WATCH_BATCH_INTERVAL_MS + 50)) - expect(sender.send).toHaveBeenCalledTimes(1) + await vi.waitFor(() => expect(sender.send).toHaveBeenCalledTimes(1), { + timeout: WATCH_BATCH_INTERVAL_MS * 20 + }) } finally { for (const teardown of watcherLifecycleState.pendingTeardowns.values()) { clearTimeout(teardown) diff --git a/src/main/ipc/filesystem/watchers/filesystem-watcher-local-unsubscribe.test.ts b/src/main/ipc/filesystem/watchers/filesystem-watcher-local-unsubscribe.test.ts index cd9d9542ab7..791bae057fe 100644 --- a/src/main/ipc/filesystem/watchers/filesystem-watcher-local-unsubscribe.test.ts +++ b/src/main/ipc/filesystem/watchers/filesystem-watcher-local-unsubscribe.test.ts @@ -82,7 +82,7 @@ describe('local filesystem watcher unsubscribe cleanup', () => { isDestroyed: () => false, send: vi.fn(), removeListener: vi.fn(), - once: vi.fn((event: string, callback: () => void) => { + on: vi.fn((event: string, callback: () => void) => { if (event === 'destroyed') { destroyedCallbacks.push(callback) } @@ -155,7 +155,7 @@ describe('local filesystem watcher unsubscribe cleanup', () => { isDestroyed: () => false, send: vi.fn(), removeListener: vi.fn(), - once: vi.fn((event: string, callback: () => void) => { + on: vi.fn((event: string, callback: () => void) => { if (event === 'destroyed') { destroyedCallbacks.push(callback) } @@ -176,7 +176,7 @@ describe('local filesystem watcher unsubscribe cleanup', () => { await watchPromise expect(unsubscribeMock).toHaveBeenCalledTimes(1) - expect(sender.once).toHaveBeenCalledWith('destroyed', expect.any(Function)) + expect(sender.on).toHaveBeenCalledWith('destroyed', expect.any(Function)) }) it('dedupes concurrent local watcher opens for the same root', async () => { @@ -228,8 +228,8 @@ describe('local filesystem watcher unsubscribe cleanup', () => { await Promise.allSettled([watchOne, watchTwo]) } - expect(senderOne.once).toHaveBeenCalledWith('destroyed', expect.any(Function)) - expect(senderTwo.once).toHaveBeenCalledWith('destroyed', expect.any(Function)) + expect(senderOne.on).toHaveBeenCalledWith('destroyed', expect.any(Function)) + expect(senderTwo.on).toHaveBeenCalledWith('destroyed', expect.any(Function)) }) it('keeps a single grace teardown timer for duplicate local unwatch calls', async () => { @@ -341,7 +341,7 @@ describe('local filesystem watcher unsubscribe cleanup', () => { isDestroyed: () => false, send: vi.fn(), removeListener: vi.fn(), - once: vi.fn((event: string, callback: () => void) => { + on: vi.fn((event: string, callback: () => void) => { if (event === 'destroyed') { destroyedCallbacks.push(callback) } @@ -383,7 +383,7 @@ describe('local filesystem watcher unsubscribe cleanup', () => { isDestroyed: () => false, send: vi.fn(), removeListener: vi.fn(), - once: vi.fn((event: string, callback: () => void) => { + on: vi.fn((event: string, callback: () => void) => { if (event === 'destroyed') { destroyedCallbacks.push(callback) } @@ -511,15 +511,15 @@ describe('local filesystem watcher unsubscribe cleanup', () => { { sender }, { worktreePath: '/tmp/repo' } ) as Promise + let watchSettled = false + void watchPromise.then(() => { + watchSettled = true + }) // Unwatch must settle before the native subscribe crawl finishes. - const watchSettledEarly = await Promise.race([ - watchPromise.then(() => true), - new Promise((resolve) => setTimeout(() => resolve(false), 20)) - ]) + await vi.waitFor(() => expect(watchSettled).toBe(true)) resolveSubscribe({ unsubscribe: unsubscribeMock }) await Promise.all([watchPromise, unwatchPromise]) - expect(watchSettledEarly).toBe(true) expect(unsubscribeMock).toHaveBeenCalledTimes(1) }) @@ -702,7 +702,7 @@ describe('local filesystem watcher unsubscribe cleanup', () => { isDestroyed: () => false, send: vi.fn(), removeListener: vi.fn(), - once: vi.fn((event: string, callback: () => void) => { + on: vi.fn((event: string, callback: () => void) => { if (event === 'destroyed') { destroyedCallbacks.push(callback) } diff --git a/src/main/ipc/filesystem/watchers/filesystem-watcher-test-sender.ts b/src/main/ipc/filesystem/watchers/filesystem-watcher-test-sender.ts index a309d080087..5ab08a8c2c3 100644 --- a/src/main/ipc/filesystem/watchers/filesystem-watcher-test-sender.ts +++ b/src/main/ipc/filesystem/watchers/filesystem-watcher-test-sender.ts @@ -1,9 +1,9 @@ import { vi, type Mock } from 'vitest' -type SenderEvents = { once: Mock; removeListener: Mock } +type SenderEvents = { on: Mock; removeListener: Mock } export function senderEvents(): SenderEvents { - return { once: vi.fn(), removeListener: vi.fn() } + return { on: vi.fn(), removeListener: vi.fn() } } export function createWatcherSender( @@ -17,10 +17,10 @@ export function createWatcherSender( return { id, isDestroyed: () => false, send, ...senderEvents() } } -/** Fires the live (registered, not removed, not yet fired) `once` handlers for `event`, as Electron would. */ +/** Fires the live (registered, not removed) `on` handlers for `event`, as Electron would. */ export function fireWatcherSenderEvent(sender: SenderEvents, event: string): void { const removed = new Set(sender.removeListener.mock.calls.map(([, callback]) => callback)) - for (const [name, callback] of sender.once.mock.calls) { + for (const [name, callback] of sender.on.mock.calls) { if (name === event && !removed.has(callback)) { sender.removeListener(name, callback) callback() diff --git a/src/main/ipc/jira/issue-summary-generation.ts b/src/main/ipc/jira/issue-summary-generation.ts index c390cae2f3b..322dc597b90 100644 --- a/src/main/ipc/jira/issue-summary-generation.ts +++ b/src/main/ipc/jira/issue-summary-generation.ts @@ -9,6 +9,7 @@ import { cancelJiraIssueSummaryRequest, generateJiraIssueSummaryRequest } from '../../text-generation/jira-issue-summary-requests' +import { addFanoutListener } from '../../event-listener-fanout' export function registerJiraIssueSummaryGenerationHandlers( store: Pick, @@ -18,7 +19,7 @@ export function registerJiraIssueSummaryGenerationHandlers( const request = GenerateIssueSummary.parse(args) const owner = `window:${event.sender.id}` const cancel = (): void => cancelJiraIssueSummaryRequest(owner, request) - event.sender.once('destroyed', cancel) + const releaseDestroyed = addFanoutListener(event.sender, 'destroyed', cancel) try { return await generateJiraIssueSummaryRequest( { @@ -29,7 +30,7 @@ export function registerJiraIssueSummaryGenerationHandlers( request ) } finally { - event.sender.removeListener('destroyed', cancel) + releaseDestroyed() } }) ipcMain.handle('jira:cancelGenerateIssueSummary', (event, args: unknown) => { diff --git a/src/main/ipc/native-chat-subscribe-lifecycle.test.ts b/src/main/ipc/native-chat-subscribe-lifecycle.test.ts index 3b39cc8c033..030067dd68d 100644 --- a/src/main/ipc/native-chat-subscribe-lifecycle.test.ts +++ b/src/main/ipc/native-chat-subscribe-lifecycle.test.ts @@ -48,7 +48,7 @@ type SenderHarness = { sender: { id: number isDestroyed: () => boolean - once: (event: string, callback: () => void) => void + on: (event: string, callback: () => void) => void removeListener: (event: string, callback: () => void) => void send: ReturnType } @@ -90,7 +90,7 @@ function createSender(id: number): SenderHarness { sender: { id, isDestroyed: () => destroyed, - once: (event, callback) => void events.once(event, callback), + on: (event, callback) => void events.on(event, callback), removeListener: (event, callback) => void events.removeListener(event, callback), send: vi.fn() } diff --git a/src/main/ipc/renderer-lifetime-abort.test.ts b/src/main/ipc/renderer-lifetime-abort.test.ts index bf704a9cbaa..630686c6cb4 100644 --- a/src/main/ipc/renderer-lifetime-abort.test.ts +++ b/src/main/ipc/renderer-lifetime-abort.test.ts @@ -86,4 +86,19 @@ describe('abortWhenRendererGone', () => { expect(sender.listenerCount('render-process-gone')).toBe(0) expect(sender.listenerCount('did-navigate')).toBe(0) }) + + it('keeps one listener per event however many lifetimes overlap on a renderer', () => { + const sender = fakeSender() + const lifetimes = Array.from({ length: 25 }, () => abortWhenRendererGone(sender)) + + for (const event of ['destroyed', 'render-process-gone', 'did-navigate']) { + expect(sender.listenerCount(event)).toBe(1) + } + lifetimes[0].dispose() + sender.emit('did-navigate', 'file:///app', 200, 'OK') + + expect(lifetimes[0].signal.aborted).toBe(false) + expect(lifetimes.slice(1).every(({ signal }) => signal.aborted)).toBe(true) + expect(sender.eventNames()).toEqual([]) + }) }) diff --git a/src/main/ipc/renderer-lifetime-abort.ts b/src/main/ipc/renderer-lifetime-abort.ts index 4e92de1fc02..de552274dd3 100644 --- a/src/main/ipc/renderer-lifetime-abort.ts +++ b/src/main/ipc/renderer-lifetime-abort.ts @@ -1,33 +1,22 @@ -import type { WebContents } from 'electron' +import { addFanoutListeners, type FanoutEventSource } from '../event-listener-fanout' -export type RendererLifetimeSender = Pick +export type RendererLifetimeSender = FanoutEventSource export const RENDERER_GONE_MESSAGE = 'The window that started this upload went away' +// Abort only committed document replacement; blocked external links and same-document navigation preserve the caller. +const RENDERER_GONE_EVENTS = ['destroyed', 'render-process-gone', 'did-navigate'] as const + /** Bind main-process work to its requesting renderer; dispose in finally to release WebContents listeners. */ export function abortWhenRendererGone(sender: RendererLifetimeSender): { signal: AbortSignal dispose: () => void } { const controller = new AbortController() - const abort = (): void => controller.abort(new Error(RENDERER_GONE_MESSAGE)) - let disposed = false - - sender.once('destroyed', abort) - sender.once('render-process-gone', abort) - // Abort only committed document replacement; blocked external links and same-document navigation preserve the caller. - sender.once('did-navigate', abort) - - return { - signal: controller.signal, - dispose: () => { - if (disposed) { - return - } - disposed = true - sender.removeListener('destroyed', abort) - sender.removeListener('render-process-gone', abort) - sender.removeListener('did-navigate', abort) - } - } + // Shared fan-out: concurrent lifetimes on one renderer add one native listener per event, not one each. + const dispose = addFanoutListeners(sender, RENDERER_GONE_EVENTS, () => { + dispose() + controller.abort(new Error(RENDERER_GONE_MESSAGE)) + }) + return { signal: controller.signal, dispose } } diff --git a/src/main/ipc/runtime-environments/runtime-environment-subscription-layout-recording.test.ts b/src/main/ipc/runtime-environments/runtime-environment-subscription-layout-recording.test.ts index 911327af77d..76c4556213a 100644 --- a/src/main/ipc/runtime-environments/runtime-environment-subscription-layout-recording.test.ts +++ b/src/main/ipc/runtime-environments/runtime-environment-subscription-layout-recording.test.ts @@ -22,7 +22,7 @@ const sender = { id: 1, isDestroyed: () => false, send: vi.fn(), - once: vi.fn(), + on: vi.fn(), removeListener: vi.fn() } const recordLayoutFrame = vi.fn() diff --git a/src/main/ipc/runtime-environments/runtime-environments-subscription-lifecycle.test.ts b/src/main/ipc/runtime-environments/runtime-environments-subscription-lifecycle.test.ts index e097127b285..a9b49955f02 100644 --- a/src/main/ipc/runtime-environments/runtime-environments-subscription-lifecycle.test.ts +++ b/src/main/ipc/runtime-environments/runtime-environments-subscription-lifecycle.test.ts @@ -152,7 +152,7 @@ describe('registerRuntimeEnvironmentHandlers', () => { id: 1, isDestroyed: () => false, send: vi.fn(), - once: vi.fn(), + on: vi.fn(), removeListener: vi.fn() } const pending = subscribe( @@ -225,7 +225,7 @@ describe('registerRuntimeEnvironmentHandlers', () => { id: 1, isDestroyed: () => false, send: (_channel: string, payload: unknown) => sent.push(payload), - once: vi.fn(), + on: vi.fn(), removeListener: destroyedListenerRemoved } }, @@ -309,7 +309,7 @@ describe('registerRuntimeEnvironmentHandlers', () => { id: 1, isDestroyed: () => false, send: vi.fn(), - once: vi.fn(), + on: vi.fn(), removeListener: vi.fn() } }, @@ -387,7 +387,7 @@ describe('registerRuntimeEnvironmentHandlers', () => { id: 1, isDestroyed: () => destroyed, send: vi.fn(), - once: vi.fn((_event: string, handler: () => void) => { + on: vi.fn((_event: string, handler: () => void) => { destroyedHandler = () => { destroyed = true handler() @@ -479,7 +479,7 @@ describe('registerRuntimeEnvironmentHandlers', () => { id: 1, isDestroyed: () => false, send: senderSend, - once: vi.fn(), + on: vi.fn(), removeListener: vi.fn() } }, @@ -551,7 +551,7 @@ describe('registerRuntimeEnvironmentHandlers', () => { id: 1, isDestroyed: () => false, send: vi.fn(), - once: vi.fn(), + on: vi.fn(), removeListener: destroyedListenerRemoved } }, diff --git a/src/main/ipc/runtime-environments/runtime-environments-subscription-routing.test.ts b/src/main/ipc/runtime-environments/runtime-environments-subscription-routing.test.ts index 21aa179348e..9298b8d7194 100644 --- a/src/main/ipc/runtime-environments/runtime-environments-subscription-routing.test.ts +++ b/src/main/ipc/runtime-environments/runtime-environments-subscription-routing.test.ts @@ -150,7 +150,7 @@ describe('registerRuntimeEnvironmentHandlers', () => { id: 1, isDestroyed: () => false, send: vi.fn(), - once: vi.fn(), + on: vi.fn(), removeListener: vi.fn() } }, @@ -162,7 +162,7 @@ describe('registerRuntimeEnvironmentHandlers', () => { id: 1, isDestroyed: () => false, send: vi.fn(), - once: vi.fn(), + on: vi.fn(), removeListener: vi.fn() } }, @@ -228,7 +228,7 @@ describe('registerRuntimeEnvironmentHandlers', () => { id: 1, isDestroyed: () => false, send: vi.fn(), - once: vi.fn(), + on: vi.fn(), removeListener: vi.fn() } }, @@ -284,7 +284,7 @@ describe('registerRuntimeEnvironmentHandlers', () => { id: 1, isDestroyed: () => false, send: senderSend, - once: vi.fn(), + on: vi.fn(), removeListener: destroyedListenerRemoved } }, @@ -358,7 +358,7 @@ describe('registerRuntimeEnvironmentHandlers', () => { id: 1, isDestroyed: () => false, send: vi.fn(), - once: vi.fn(), + on: vi.fn(), removeListener: vi.fn() } }, diff --git a/src/main/ipc/runtime-environments/runtime-environments-subscription-teardown.test.ts b/src/main/ipc/runtime-environments/runtime-environments-subscription-teardown.test.ts index f1168f74641..54d86bdb2af 100644 --- a/src/main/ipc/runtime-environments/runtime-environments-subscription-teardown.test.ts +++ b/src/main/ipc/runtime-environments/runtime-environments-subscription-teardown.test.ts @@ -163,7 +163,7 @@ describe('registerRuntimeEnvironmentHandlers', () => { id: 1, isDestroyed: () => false, send: vi.fn(), - once: vi.fn(), + on: vi.fn(), removeListener: destroyedListenerRemoved } }, @@ -235,7 +235,7 @@ describe('registerRuntimeEnvironmentHandlers', () => { id: 1, isDestroyed: () => false, send: senderSend, - once: vi.fn(), + on: vi.fn(), removeListener: vi.fn() } }, @@ -301,7 +301,7 @@ describe('registerRuntimeEnvironmentHandlers', () => { id: 1, isDestroyed: () => false, send: senderSend, - once: vi.fn(), + on: vi.fn(), removeListener: vi.fn() } } @@ -379,7 +379,7 @@ describe('registerRuntimeEnvironmentHandlers', () => { id: 1, isDestroyed: () => false, send: senderSend, - once: vi.fn(), + on: vi.fn(), removeListener: vi.fn() } }, @@ -450,7 +450,7 @@ describe('registerRuntimeEnvironmentHandlers', () => { return false }, send: senderSend, - once: vi.fn(), + on: vi.fn(), removeListener: vi.fn() } } @@ -514,7 +514,7 @@ describe('registerRuntimeEnvironmentHandlers', () => { id: 1, isDestroyed: () => false, send: senderSend, - once: vi.fn(), + on: vi.fn(), removeListener: vi.fn() } }, diff --git a/src/main/ipc/runtime-ssh-access-handlers.ts b/src/main/ipc/runtime-ssh-access-handlers.ts index 454c3e53fbd..30d5502e30a 100644 --- a/src/main/ipc/runtime-ssh-access-handlers.ts +++ b/src/main/ipc/runtime-ssh-access-handlers.ts @@ -4,6 +4,7 @@ import { RuntimeSshAccessUnlinkRequestSchema } from '../../shared/runtime-ssh-access' import { linkRuntimeSshAccess, unlinkRuntimeSshAccess } from '../ssh/runtime-ssh-access' +import { addFanoutListener } from '../event-listener-fanout' export function registerRuntimeSshAccessHandlers(options: { getUserDataPath: () => string @@ -14,14 +15,14 @@ export function registerRuntimeSshAccessHandlers(options: { // Unreachable links hold two lifecycle queues; cancel when their window closes. const controller = new AbortController() const cancel = (): void => controller.abort() - event.sender.once('destroyed', cancel) + const releaseDestroyed = addFanoutListener(event.sender, 'destroyed', cancel) try { return await linkRuntimeSshAccess(options.getUserDataPath(), args, { signal: controller.signal, invalidateTransport: options.invalidateTransport }) } finally { - event.sender.removeListener('destroyed', cancel) + releaseDestroyed() } }) ipcMain.handle('runtimeEnvironments:unlinkSshAccess', async (_event, input: unknown) => { diff --git a/src/main/ipc/workspace-cleanup-snapshot-ipc.test.ts b/src/main/ipc/workspace-cleanup-snapshot-ipc.test.ts index e6d2040368b..628940b9723 100644 --- a/src/main/ipc/workspace-cleanup-snapshot-ipc.test.ts +++ b/src/main/ipc/workspace-cleanup-snapshot-ipc.test.ts @@ -43,12 +43,13 @@ import { registerWorkspaceCleanupHandlers } from './workspace-cleanup' const NOW = 1_700_000_000_000 function makeScanEvent(senderOverrides: Record = {}): never { + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the handler reads only the sender members supplied here. return { sender: { id: 1, isDestroyed: () => false, send: vi.fn(), - once: vi.fn(), + on: vi.fn(), removeListener: vi.fn(), ...senderOverrides } @@ -164,7 +165,7 @@ describe('workspace cleanup snapshot IPC', () => { .mocked(ipcMain.handle) .mock.calls.find(([channel]) => channel === 'workspaceCleanup:scan')?.[1] const event = makeScanEvent({ - once: vi.fn((eventName: string, listener: () => void) => { + on: vi.fn((eventName: string, listener: () => void) => { if (eventName === 'destroyed') { abortListener = listener } diff --git a/src/main/ipc/workspace-cleanup.ts b/src/main/ipc/workspace-cleanup.ts index 55c12a53cbb..d52cba21944 100644 --- a/src/main/ipc/workspace-cleanup.ts +++ b/src/main/ipc/workspace-cleanup.ts @@ -11,6 +11,7 @@ import { import { parseExecutionHostId } from '../../shared/execution-host' import { getWorkspaceCleanupHostIdentity } from '../../shared/workspace-cleanup-host-identity' import { scanWorkspaceCleanup } from './workspace-cleanup-scan' +import { addFanoutListener } from '../event-listener-fanout' import { hasTargetedWorkspaceCleanupScan } from './workspace-cleanup-scan-targets' import { persistWorkspaceCleanupScanResult, @@ -63,7 +64,7 @@ export function registerWorkspaceCleanupHandlers(store: Store): void { } // Window retirement must cancel scan work, not merely mute its progress. const onSenderDestroyed = (): void => controller.abort() - sender.once('destroyed', onSenderDestroyed) + const releaseDestroyed = addFanoutListener(sender, 'destroyed', onSenderDestroyed) try { const result = await scanWorkspaceCleanup(store, scanArgs, { signal: controller.signal, @@ -81,9 +82,7 @@ export function registerWorkspaceCleanupHandlers(store: Store): void { } return result } finally { - if (!sender.isDestroyed()) { - sender.removeListener('destroyed', onSenderDestroyed) - } + releaseDestroyed() if (scanKey && activeScans.get(scanKey) === controller) { activeScans.delete(scanKey) } diff --git a/src/main/media/media-preview-protocol.test.ts b/src/main/media/media-preview-protocol.test.ts index c005ba75eff..bb51913ccbc 100644 --- a/src/main/media/media-preview-protocol.test.ts +++ b/src/main/media/media-preview-protocol.test.ts @@ -23,7 +23,7 @@ describe('media preview protocol', () => { const event = { sender: { id: 24859, - once: vi.fn().mockImplementation((name, callback) => emitter.once(name, callback)), + on: vi.fn().mockImplementation((name, callback) => emitter.on(name, callback)), removeListener: vi .fn() .mockImplementation((name, callback) => emitter.removeListener(name, callback)) diff --git a/src/main/media/media-preview-protocol.ts b/src/main/media/media-preview-protocol.ts index c46799e3527..9936d324cd3 100644 --- a/src/main/media/media-preview-protocol.ts +++ b/src/main/media/media-preview-protocol.ts @@ -20,7 +20,7 @@ export const MEDIA_PREVIEW_CUSTOM_SCHEME: Electron.CustomScheme = { } export function readMediaPreview( - event: { sender: Pick }, + event: { sender: Pick }, target: MediaTarget, store: Store ): { content: string; isBinary: boolean; mimeType: string; mediaUrl: string } | null { diff --git a/src/main/window/dropped-path-preparation-ipc.test.ts b/src/main/window/dropped-path-preparation-ipc.test.ts index 87a9192eea0..2a6f2e3b112 100644 --- a/src/main/window/dropped-path-preparation-ipc.test.ts +++ b/src/main/window/dropped-path-preparation-ipc.test.ts @@ -69,7 +69,7 @@ function createWindow() { const webContents = { isDestroyed: () => destroyed, send: vi.fn(), - once: vi.fn<(event: string, listener: () => void) => void>(), + on: vi.fn<(event: string, listener: () => void) => void>(), removeListener: vi.fn() } const fake = { @@ -88,7 +88,7 @@ function createWindow() { }, destroy: () => { destroyed = true - webContents.once.mock.calls.find(([event]) => event === 'destroyed')?.[1]() + webContents.on.mock.calls.find(([event]) => event === 'destroyed')?.[1]() } } } diff --git a/src/main/window/project-move-pause.test.ts b/src/main/window/project-move-pause.test.ts index 4fb9600e369..7950b08f6cc 100644 --- a/src/main/window/project-move-pause.test.ts +++ b/src/main/window/project-move-pause.test.ts @@ -89,6 +89,25 @@ describe('main-owned project move pause', () => { expect(f.webContents.eventNames()).toEqual([]) }) + it('shares one lifecycle listener across overlapping pauses and releases it after', async () => { + const f = fixture() + const paused = Array.from({ length: 12 }, () => + pauseWorkspacesForProjectMove(f.mainWindow, f, ['wt']) + ) + + expect(ipc.listenerCount('ui:projectMovePauseResponse')).toBe(1) + expect(f.window.listenerCount('closed')).toBe(1) + for (const event of ['destroyed', 'render-process-gone', 'did-start-navigation']) { + expect(f.webContents.listenerCount(event)).toBe(1) + } + f.webContents.emit('render-process-gone') + + await Promise.all(paused.map((pause) => expect(pause).rejects.toThrow('renderer_unavailable'))) + expect(f.window.eventNames()).toEqual([]) + expect(f.webContents.eventNames()).toEqual([]) + expect(ipc.listenerCount('ui:projectMovePauseResponse')).toBe(0) + }) + it('does not stop hosts when an old preload or follower never answers the handshake', async () => { const f = fixture() const paused = pauseWorkspacesForProjectMove(f.mainWindow, f, ['wt']) diff --git a/src/main/window/project-move-pause.ts b/src/main/window/project-move-pause.ts index 1b81a49ce44..034a0089474 100644 --- a/src/main/window/project-move-pause.ts +++ b/src/main/window/project-move-pause.ts @@ -7,6 +7,7 @@ import { type ProjectMovePauseResponse } from '../../shared/project-move-pause' import type { WorkspaceSleepOutcome, WorkspaceSleepRequest } from '../../shared/workspace-sleep' +import { addFanoutListener, addFanoutListeners } from '../event-listener-fanout' type SleepRuntime = { sleepWorkspaces: (request: WorkspaceSleepRequest) => Promise @@ -101,11 +102,12 @@ export async function pauseWorkspacesForProjectMove( } }) } - ipcMain.on('ui:projectMovePauseResponse', onResponse) - mainWindow.once('closed', onUnavailable) - webContents.once('destroyed', onUnavailable) - webContents.once('render-process-gone', onUnavailable) - webContents.on('did-start-navigation', onNavigation) + const releaseListeners = [ + addFanoutListener(ipcMain, 'ui:projectMovePauseResponse', onResponse), + addFanoutListener(mainWindow, 'closed', onUnavailable), + addFanoutListeners(webContents, ['destroyed', 'render-process-gone'], onUnavailable), + addFanoutListener(webContents, 'did-start-navigation', onNavigation) + ] const envelope = { requestId, protocol: PROJECT_MOVE_PAUSE_PROTOCOL } as const let outcomes: WorkspaceSleepOutcome[] = [] try { @@ -154,10 +156,8 @@ export async function pauseWorkspacesForProjectMove( /* The renderer no longer holds a live reservation. */ } } - ipcMain.removeListener('ui:projectMovePauseResponse', onResponse) - mainWindow.removeListener('closed', onUnavailable) - webContents.removeListener('destroyed', onUnavailable) - webContents.removeListener('render-process-gone', onUnavailable) - webContents.removeListener('did-start-navigation', onNavigation) + for (const release of releaseListeners) { + release() + } } } diff --git a/src/main/window/session-tab-close-request-relay.test.ts b/src/main/window/session-tab-close-request-relay.test.ts index d875a6e492f..fe13c0c8397 100644 --- a/src/main/window/session-tab-close-request-relay.test.ts +++ b/src/main/window/session-tab-close-request-relay.test.ts @@ -118,4 +118,64 @@ describe('requestSessionTabCloseFromRenderer', () => { await rejection expect(ipcEmitter.listenerCount('ui:sessionTabCloseResponse')).toBe(0) }) + + it('shares one lifecycle listener across overlapping close requests', async () => { + const { requestSessionTabCloseFromRenderer } = await import('./session-tab-close-request-relay') + const { mainWindow, webContents } = createMainWindow() + const pending = Array.from({ length: 15 }, (_, index) => + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the EventEmitter double implements the BrowserWindow members the relay uses. + requestSessionTabCloseFromRenderer(mainWindow as never, `tab-${index}`, 'wt-1') + ) + + expect(ipcEmitter.listenerCount('ui:sessionTabCloseResponse')).toBe(1) + expect(mainWindow.listenerCount('closed')).toBe(1) + for (const event of ['destroyed', 'render-process-gone', 'did-start-loading']) { + expect(webContents.listenerCount(event)).toBe(1) + } + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the relay sends only session tab close requests on this double. + const requests = webContents.send.mock.calls as [string, { requestId: string }][] + for (const [, request] of requests) { + ipcEmitter.emit( + 'ui:sessionTabCloseResponse', + { sender: webContents }, + { requestId: request.requestId } + ) + } + + await Promise.all(pending) + expect(mainWindow.eventNames()).toEqual([]) + expect(webContents.eventNames()).toEqual([]) + expect(ipcEmitter.listenerCount('ui:sessionTabCloseResponse')).toBe(0) + }) + + it.each([ + ['window', 'closed'], + ['webContents', 'destroyed'], + ['webContents', 'did-start-loading'] + ] as const)( + 'rejects both overlapping requests on one %s %s and returns to baseline listeners', + async (target, eventName) => { + const { requestSessionTabCloseFromRenderer } = + await import('./session-tab-close-request-relay') + const { mainWindow, webContents } = createMainWindow() + const baseline = { + window: mainWindow.eventNames(), + webContents: webContents.eventNames(), + ipc: ipcEmitter.listenerCount('ui:sessionTabCloseResponse') + } + const pending = ['tab-1', 'tab-2'].map((tabId) => + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the EventEmitter double implements the BrowserWindow members the relay uses. + requestSessionTabCloseFromRenderer(mainWindow as never, tabId, 'wt-1') + ) + + ;(target === 'window' ? mainWindow : webContents).emit(eventName) + + await Promise.all( + pending.map((request) => expect(request).rejects.toThrow('renderer_unavailable')) + ) + expect(mainWindow.eventNames()).toEqual(baseline.window) + expect(webContents.eventNames()).toEqual(baseline.webContents) + expect(ipcEmitter.listenerCount('ui:sessionTabCloseResponse')).toBe(baseline.ipc) + } + ) }) diff --git a/src/main/window/session-tab-close-request-relay.ts b/src/main/window/session-tab-close-request-relay.ts index 07bec3d5ce6..834da75f24f 100644 --- a/src/main/window/session-tab-close-request-relay.ts +++ b/src/main/window/session-tab-close-request-relay.ts @@ -7,6 +7,7 @@ import type { SessionTabCloseResponse } from '../../shared/session-tab-close' import { SESSION_TAB_CLOSE_TIMEOUT_ERROR } from '../../shared/session-tab-close' +import { addFanoutListener, addFanoutListeners } from '../event-listener-fanout' const SESSION_TAB_CLOSE_CONFIRMATION_MS = 5 * 60_000 const SESSION_TAB_CLOSE_RESPONSE_GRACE_MS = 5_000 @@ -23,17 +24,16 @@ export async function requestSessionTabCloseFromRenderer( const requestId = randomUUID() await new Promise((resolve, reject) => { let settled = false + const releaseListeners: (() => void)[] = [] const finish = (error?: Error): void => { if (settled) { return } settled = true clearTimeout(timeout) - ipcMain.removeListener('ui:sessionTabCloseResponse', onResponse) - mainWindow.removeListener('closed', onRendererUnavailable) - webContents.removeListener('destroyed', onRendererUnavailable) - webContents.removeListener('render-process-gone', onRendererUnavailable) - webContents.removeListener('did-start-loading', onRendererUnavailable) + for (const release of releaseListeners) { + release() + } if (error) { reject(error) } else { @@ -53,11 +53,15 @@ export async function requestSessionTabCloseFromRenderer( SESSION_TAB_CLOSE_CONFIRMATION_MS + SESSION_TAB_CLOSE_RESPONSE_GRACE_MS ) timeout.unref?.() - ipcMain.on('ui:sessionTabCloseResponse', onResponse) - mainWindow.once('closed', onRendererUnavailable) - webContents.once('destroyed', onRendererUnavailable) - webContents.once('render-process-gone', onRendererUnavailable) - webContents.once('did-start-loading', onRendererUnavailable) + releaseListeners.push( + addFanoutListener(ipcMain, 'ui:sessionTabCloseResponse', onResponse), + addFanoutListener(mainWindow, 'closed', onRendererUnavailable), + addFanoutListeners( + webContents, + ['destroyed', 'render-process-gone', 'did-start-loading'], + onRendererUnavailable + ) + ) const request: SessionTabCloseRequest = { requestId, tabId, worktreeId, expiresAt } try { webContents.send('ui:sessionTabCloseRequest', request) diff --git a/src/main/window/terminal-tab-close-request-relay.test.ts b/src/main/window/terminal-tab-close-request-relay.test.ts index ab3e3c57c77..7f2c9213125 100644 --- a/src/main/window/terminal-tab-close-request-relay.test.ts +++ b/src/main/window/terminal-tab-close-request-relay.test.ts @@ -110,4 +110,31 @@ describe('requestTerminalTabCloseFromRenderer', () => { expect(webContents.listenerCount('destroyed')).toBe(0) expect(webContents.listenerCount('render-process-gone')).toBe(0) }) + + it('shares one lifecycle listener across overlapping close requests', async () => { + const { requestTerminalTabCloseFromRenderer } = + await import('./terminal-tab-close-request-relay') + const webContents = Object.assign(new EventEmitter(), { + isDestroyed: () => false, + send: vi.fn() + }) + const mainWindow = Object.assign(new EventEmitter(), { isDestroyed: () => false, webContents }) + const pending = Array.from({ length: 15 }, (_, index) => + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the EventEmitter double implements the BrowserWindow members the relay uses. + requestTerminalTabCloseFromRenderer(mainWindow as never, `tab-${index}`) + ) + + expect(ipcEmitter.listenerCount('ui:terminalTabCloseResponse')).toBe(1) + expect(mainWindow.listenerCount('closed')).toBe(1) + expect(webContents.listenerCount('destroyed')).toBe(1) + expect(webContents.listenerCount('render-process-gone')).toBe(1) + webContents.emit('render-process-gone') + + await Promise.all( + pending.map((request) => expect(request).rejects.toThrow('renderer_unavailable')) + ) + expect(mainWindow.eventNames()).toEqual([]) + expect(webContents.eventNames()).toEqual([]) + expect(ipcEmitter.listenerCount('ui:terminalTabCloseResponse')).toBe(0) + }) }) diff --git a/src/main/window/terminal-tab-close-request-relay.ts b/src/main/window/terminal-tab-close-request-relay.ts index ce2d801c90b..b3b7d6d7a65 100644 --- a/src/main/window/terminal-tab-close-request-relay.ts +++ b/src/main/window/terminal-tab-close-request-relay.ts @@ -6,6 +6,7 @@ import type { TerminalTabCloseRequest, TerminalTabCloseResponse } from '../../shared/terminal-tab-close' +import { addFanoutListener, addFanoutListeners } from '../event-listener-fanout' const TERMINAL_TAB_CLOSE_TIMEOUT_MS = 20_000 @@ -24,6 +25,7 @@ export async function requestTerminalTabCloseFromRenderer( const requestId = randomUUID() await new Promise((resolve, reject) => { let settled = false + const releaseListeners: (() => void)[] = [] const onRendererUnavailable = (): void => finish(new Error('renderer_unavailable')) const finish = (error?: Error): void => { if (settled) { @@ -31,13 +33,8 @@ export async function requestTerminalTabCloseFromRenderer( } settled = true clearTimeout(timeout) - ipcMain.removeListener('ui:terminalTabCloseResponse', onResponse) - if (typeof mainWindow.removeListener === 'function') { - mainWindow.removeListener('closed', onRendererUnavailable) - } - if (typeof webContents.removeListener === 'function') { - webContents.removeListener('destroyed', onRendererUnavailable) - webContents.removeListener('render-process-gone', onRendererUnavailable) + for (const release of releaseListeners) { + release() } if (error) { reject(error) @@ -60,13 +57,15 @@ export async function requestTerminalTabCloseFromRenderer( finish() } } - ipcMain.on('ui:terminalTabCloseResponse', onResponse) - if (typeof mainWindow.once === 'function') { - mainWindow.once('closed', onRendererUnavailable) + releaseListeners.push(addFanoutListener(ipcMain, 'ui:terminalTabCloseResponse', onResponse)) + // Lean test windows may lack event methods. + if (typeof mainWindow.on === 'function') { + releaseListeners.push(addFanoutListener(mainWindow, 'closed', onRendererUnavailable)) } - if (typeof webContents.once === 'function') { - webContents.once('destroyed', onRendererUnavailable) - webContents.once('render-process-gone', onRendererUnavailable) + if (typeof webContents.on === 'function') { + releaseListeners.push( + addFanoutListeners(webContents, ['destroyed', 'render-process-gone'], onRendererUnavailable) + ) } const request: TerminalTabCloseRequest = { requestId, tabId, ...options } try {