Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 5 additions & 14 deletions src/preload/api/devDebug.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { ipcRenderer } from 'electron'

import type { DevDebugConfig, PasteDebugSession } from '@preload/api/types.js'
import type { RenderShapeAppendResult } from '@shared/types/renderShapes.js'
import { subscribeShared } from '@preload/api/ipc.js'

export const devDebugApi = {
getDevDebugConfig: (): Promise<DevDebugConfig> =>
Expand Down Expand Up @@ -55,16 +56,11 @@ export const devDebugApi = {
// event — for an idle restored pane that is whenever the user first
// prompts it, unboundedly after Feed mount, so every renderer-side poll
// schedule loses the race. Same subscribe shape as lsp:diagnostics.
// Shared (#1015/#1039 review): every Feed mounts a RenderShapeCaptureProvider
// that subscribes, so 11 rendered panes crossed MaxListeners.
onSessionRecordingStarted: (
cb: (payload: { sessionId: string; generation: string }) => void,
): (() => void) => {
const listener = (
_evt: unknown,
payload: { sessionId: string; generation: string },
): void => cb(payload)
ipcRenderer.on('record-session:started', listener)
return () => ipcRenderer.removeListener('record-session:started', listener)
},
): (() => void) => subscribeShared('record-session:started', cb),
// Natural provider exit is a two-step close: main keeps the recorder open
// for a short grace window and asks the renderer to flush its coalesced
// shape counters. The renderer acknowledges by calling finish below; main's
Expand All @@ -76,12 +72,7 @@ export const devDebugApi = {
// closing fresh state.
onSessionRecordingStopping: (
cb: (payload: { sessionId: string; generation?: string }) => void,
): (() => void) => {
const listener = (_evt: unknown, payload: { sessionId: string; generation?: string }): void =>
cb(payload)
ipcRenderer.on('record-session:stopping', listener)
return () => ipcRenderer.removeListener('record-session:stopping', listener)
},
): (() => void) => subscribeShared('record-session:stopping', cb),
finishSessionRecordingStop: (sessionId: string, generation?: string): Promise<void> =>
ipcRenderer.invoke('record-session:finish-stop', sessionId, generation),
readRenderShapeSightings: (): Promise<{
Expand Down
5 changes: 3 additions & 2 deletions src/preload/api/dictation.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { ipcRenderer } from 'electron'

import { subscribe } from '@preload/api/ipc.js'
import { subscribe, subscribeShared } from '@preload/api/ipc.js'
import type {
DictationApiKeyStatus,
DictationApiKeySetResult,
Expand Down Expand Up @@ -31,8 +31,9 @@ export const dictationApi = {
onDictationHotkeyUp: (handler: (payload: { binding: string }) => void): Unsub =>
subscribe('dictation:hotkey-up', handler),

// Shared (#1015): every mounted composer subscribes.
onDictationStreamTranscript: (handler: (payload: DictationStreamTranscriptEvent) => void): Unsub =>
subscribe('dictation:stream-transcript', handler),
subscribeShared('dictation:stream-transcript', handler),

startDictationStream: (params: {
provider: DictationProvider
Expand Down
9 changes: 4 additions & 5 deletions src/preload/api/goalLoop.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { ipcRenderer } from 'electron'
import { subscribeShared } from '@preload/api/ipc.js'
import type { GoalLoopControlAction, GoalLoopState } from '@shared/types/goalLoop.js'

export type GoalLoopControlRequest = {
Expand All @@ -12,9 +13,7 @@ export const goalLoopApi = {
ipcRenderer.invoke('goal-loop:read', sessionIds),
controlGoalLoop: (request: GoalLoopControlRequest): Promise<GoalLoopState | null> =>
ipcRenderer.invoke('goal-loop:control', request),
onGoalLoopChanged: (listener: () => void): (() => void) => {
const handler = () => listener()
ipcRenderer.on('goal-loop:changed', handler)
return () => { ipcRenderer.removeListener('goal-loop:changed', handler) }
},
// Shared (#1015): every mounted GoalLoopPane subscribes.
onGoalLoopChanged: (listener: () => void): (() => void) =>
subscribeShared('goal-loop:changed', () => listener()),
}
62 changes: 58 additions & 4 deletions src/preload/api/ipc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,70 @@ import type { Unsub } from '@preload/api/types.js'
// and returns the resulting Unsub. This keeps domain modules tiny —
// they don't each reimplement "add a listener, return a remover."
//
// Why one listener per caller (not multiplexed):
// Why one listener per caller (not multiplexed) by default:
// Most onX consumers subscribe once at app mount with a single
// callback that dispatches by sessionId. ipcRenderer.on fans the
// event out to every registered listener cheaply; we don't need a
// dedupe layer here. The ONE case that needs multiplexing is LSP
// diagnostics (see ./lsp.ts) — a set-of-subscribers pattern that
// survives hot-module reloads without leaking N IPC listeners.
// dedupe layer for those. Channels that every mounted PANE subscribes
// to use subscribeShared below instead.

export function subscribe<T>(channel: string, cb: (payload: T) => void): Unsub {
const listener = (_evt: unknown, payload: T) => cb(payload)
ipcRenderer.on(channel, listener)
return () => ipcRenderer.removeListener(channel, listener)
}

/**
* One ipcRenderer listener per channel, fanned out to every subscriber.
*
* WHY (#1015): some channels are subscribed once per mounted pane
* (goal-loop:changed from GoalLoopPane, dictation:stream-transcript from each
* composer). Eleven panes crossed Node's default of 10 listeners, and the dev
* app warned "Possible EventEmitter memory leak" at every startup. It was not
* a leak (every pane unsubscribes), but a permanent false alarm hides the
* next real one. Raising MaxListeners would silence exactly that alarm.
* LSP diagnostics (./lsp.ts) solved the same problem with its own Set and keeps
* it on purpose: its listener stays installed for the renderer's lifetime,
* because code blocks mount and unmount constantly while scrolling. This one
* removes the relay with its last subscriber.
*
* The relay is removed with the last subscriber, so no listener outlives its
* users. Each subscriber is isolated: one pane that throws must not stop the
* others from hearing the event, which separate ipcRenderer listeners did not
* guarantee either (EventEmitter stops at the first throw).
*/
type SharedChannel = { subscribers: Set<(payload: unknown) => void>; relay: (event: unknown, payload: unknown) => void }
const sharedChannels = new Map<string, SharedChannel>()

export function subscribeShared<T>(channel: string, cb: (payload: T) => void): Unsub {
let shared = sharedChannels.get(channel)
if (!shared) {
const subscribers = new Set<(payload: unknown) => void>()
const relay = (_event: unknown, payload: unknown) => {
// Copied first: a subscriber may unsubscribe (a pane unmounting in
// response to the event) while the loop runs.
for (const subscriber of [...subscribers]) {
try {
subscriber(payload)
} catch (error) {
console.error(`[ipc] a ${channel} subscriber threw:`, error)
}
}
}
ipcRenderer.on(channel, relay)
shared = { subscribers, relay }
sharedChannels.set(channel, shared)
}
// A fresh wrapper per call: the same callback subscribed twice is two
// subscriptions, each removed by its own Unsub, as with ipcRenderer.on.
const subscription = (payload: unknown) => cb(payload as T)
shared.subscribers.add(subscription)
return () => {
const current = sharedChannels.get(channel)
if (!current || !current.subscribers.delete(subscription)) return
if (current.subscribers.size === 0) {
ipcRenderer.removeListener(channel, current.relay)
sharedChannels.delete(channel)
}
}
}
65 changes: 65 additions & 0 deletions src/preload/api/sharedListeners.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
import { EventEmitter } from 'node:events'
import { afterEach, describe, expect, it, vi } from 'vitest'

// #1015: at startup the dev app warned "11 goal-loop:changed listeners added
// to [IpcRenderer]. MaxListeners is 10", and the same for
// dictation:stream-transcript. It is not a leak: every mounted pane
// subscribes once (GoalLoopPane, the composer's dictation hook) and cleans up.
// But each subscription was its own ipcRenderer listener, so 11 panes crossed
// Node's warning threshold, and that noise hides the next real leak.
//
// ipcRenderer is a real EventEmitter here, so listenerCount and the
// MaxListeners warning behave exactly as in Electron's renderer.
const ipc = vi.hoisted(() => ({ renderer: null as unknown as EventEmitter }))
vi.mock('electron', async () => {
const { EventEmitter: Emitter } = await import('node:events')
ipc.renderer = new Emitter()
return { ipcRenderer: ipc.renderer }
})

const { goalLoopApi } = await import('./goalLoop.js')
const { dictationApi } = await import('./dictation.js')
const { devDebugApi } = await import('./devDebug.js')
const { tldrApi } = await import('./tldr.js')

afterEach(() => { vi.restoreAllMocks() })

describe.each([
['goal-loop:changed', (cb: (payload: unknown) => void) => goalLoopApi.onGoalLoopChanged(() => cb(undefined))],
['dictation:stream-transcript', (cb: (payload: unknown) => void) => dictationApi.onDictationStreamTranscript(cb as never)],
['record-session:started', (cb: (payload: unknown) => void) => devDebugApi.onSessionRecordingStarted(cb as never)],
['record-session:stopping', (cb: (payload: unknown) => void) => devDebugApi.onSessionRecordingStopping(cb as never)],
['tldr:changed', (cb: (payload: unknown) => void) => tldrApi.onTldrChanged(cb as never)],
['goal:changed', (cb: (payload: unknown) => void) => tldrApi.onGoalChanged(cb as never)],
])('%s', (channel, subscribe) => {
it('many panes share ONE ipcRenderer listener, every pane still hears every event, and the last unsubscribe removes it', async () => {
const warning = vi.fn()
process.on('warning', warning)
const received = Array.from({ length: 12 }, () => vi.fn())
const unsubscribes = received.map(cb => subscribe(cb))
expect(ipc.renderer.listenerCount(channel)).toBe(1)

ipc.renderer.emit(channel, {}, { sessionId: 's1' })
for (const cb of received) expect(cb).toHaveBeenCalledTimes(1)

unsubscribes.slice(0, 11).forEach(unsubscribe => unsubscribe())
expect(ipc.renderer.listenerCount(channel)).toBe(1)
unsubscribes[11]!()
expect(ipc.renderer.listenerCount(channel)).toBe(0)
// Node emits MaxListenersExceededWarning on process.nextTick, so the spy
// must outlive a tick or this check can never fail (#1039 review).
await new Promise(resolve => setImmediate(resolve))
process.off('warning', warning)
expect(warning).not.toHaveBeenCalled()
})

it('one subscriber that throws does not starve the others', () => {
vi.spyOn(console, 'error').mockImplementation(() => undefined)
const after = vi.fn()
const off1 = subscribe(() => { throw new Error('pane crashed') })
const off2 = subscribe(after)
ipc.renderer.emit(channel, {}, {})
expect(after).toHaveBeenCalledTimes(1)
off1(); off2()
})
})
17 changes: 7 additions & 10 deletions src/preload/api/tldr.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { ipcRenderer } from 'electron'
import { subscribeShared } from '@preload/api/ipc.js'
import type { TldrEnforcementStatus, TldrHistoryEntry, TldrRecord, TldrUpdate } from '@shared/types/tldr.js'

export const tldrApi = {
Expand All @@ -13,15 +14,11 @@ export const tldrApi = {
readTldrHistory: (identity: string): Promise<TldrHistoryEntry[]> => ipcRenderer.invoke('tldr:history', identity),
readGoals: (identities: string[]): Promise<Record<string, TldrRecord>> => ipcRenderer.invoke('goal:read', identities),
readGoalHistory: (identity: string): Promise<TldrHistoryEntry[]> => ipcRenderer.invoke('goal:history', identity),
onGoalChanged: (listener: (update: TldrUpdate) => void): (() => void) => {
const handler = (_event: Electron.IpcRendererEvent, update: TldrUpdate) => listener(update)
ipcRenderer.on('goal:changed', handler)
return () => { ipcRenderer.removeListener('goal:changed', handler) }
},
// Shared (#1039 review): every visible pane's peek subscribes while a
// TLDR/Goal peek is up.
onGoalChanged: (listener: (update: TldrUpdate) => void): (() => void) =>
subscribeShared('goal:changed', listener),
readTldrEnforcement: (identities: string[]): Promise<Record<string, TldrEnforcementStatus>> => ipcRenderer.invoke('tldr:enforcement', identities),
onTldrChanged: (listener: (update: TldrUpdate) => void): (() => void) => {
const handler = (_event: Electron.IpcRendererEvent, update: TldrUpdate) => listener(update)
ipcRenderer.on('tldr:changed', handler)
return () => { ipcRenderer.removeListener('tldr:changed', handler) }
},
onTldrChanged: (listener: (update: TldrUpdate) => void): (() => void) =>
subscribeShared('tldr:changed', listener),
}
Loading