mirror of
https://github.com/stablyai/orca.git
synced 2026-10-31 00:57:45 +00:00
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
This commit is contained in:
@@ -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([])
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,82 @@
|
||||
/** The listener surface of Node's EventEmitter, which WebContents, BrowserWindow and ipcMain share. */
|
||||
export type FanoutEventSource = Pick<NodeJS.EventEmitter, 'on' | 'removeListener'>
|
||||
|
||||
// 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<FanoutCallback>
|
||||
}
|
||||
|
||||
const fanoutsBySource = new WeakMap<FanoutEventSource, Map<string, EventFanout>>()
|
||||
|
||||
/**
|
||||
* 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<FanoutCallback>()
|
||||
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()
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -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)
|
||||
|
||||
@@ -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<unknown>
|
||||
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<false>((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)
|
||||
}
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -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<Store, 'getSettings'>,
|
||||
@@ -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) => {
|
||||
|
||||
@@ -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<typeof vi.fn>
|
||||
}
|
||||
@@ -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()
|
||||
}
|
||||
|
||||
@@ -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([])
|
||||
})
|
||||
})
|
||||
|
||||
@@ -1,33 +1,22 @@
|
||||
import type { WebContents } from 'electron'
|
||||
import { addFanoutListeners, type FanoutEventSource } from '../event-listener-fanout'
|
||||
|
||||
export type RendererLifetimeSender = Pick<WebContents, 'once' | 'removeListener'>
|
||||
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 }
|
||||
}
|
||||
|
||||
+1
-1
@@ -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()
|
||||
|
||||
+6
-6
@@ -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
|
||||
}
|
||||
},
|
||||
|
||||
@@ -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()
|
||||
}
|
||||
},
|
||||
|
||||
+6
-6
@@ -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()
|
||||
}
|
||||
},
|
||||
|
||||
@@ -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) => {
|
||||
|
||||
@@ -43,12 +43,13 @@ import { registerWorkspaceCleanupHandlers } from './workspace-cleanup'
|
||||
const NOW = 1_700_000_000_000
|
||||
|
||||
function makeScanEvent(senderOverrides: Record<string, unknown> = {}): 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
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
@@ -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))
|
||||
|
||||
@@ -20,7 +20,7 @@ export const MEDIA_PREVIEW_CUSTOM_SCHEME: Electron.CustomScheme = {
|
||||
}
|
||||
|
||||
export function readMediaPreview(
|
||||
event: { sender: Pick<WebContents, 'id' | 'once' | 'removeListener'> },
|
||||
event: { sender: Pick<WebContents, 'id' | 'on' | 'removeListener'> },
|
||||
target: MediaTarget,
|
||||
store: Store
|
||||
): { content: string; isBinary: boolean; mimeType: string; mediaUrl: string } | null {
|
||||
|
||||
@@ -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]()
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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'])
|
||||
|
||||
@@ -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<WorkspaceSleepOutcome[]>
|
||||
@@ -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()
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
)
|
||||
})
|
||||
|
||||
@@ -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<void>((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)
|
||||
|
||||
@@ -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)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -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<void>((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 {
|
||||
|
||||
Reference in New Issue
Block a user