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 {