fix: clear stale pty orphan cleanup listener (#4200)

This commit is contained in:
Neil
2026-05-31 08:05:07 -07:00
committed by GitHub
parent 195dbda186
commit acda1b2b5b
2 changed files with 57 additions and 3 deletions
+46
View File
@@ -3934,6 +3934,52 @@ describe('registerPtyHandlers', () => {
)
})
it('removes the previous orphan-cleanup listener from its original webContents', () => {
const firstWindow = {
isDestroyed: () => false,
webContents: {
on: vi.fn(),
send: vi.fn(),
removeListener: vi.fn()
}
}
const secondWindow = {
isDestroyed: () => false,
webContents: {
on: vi.fn(),
send: vi.fn(),
removeListener: vi.fn()
}
}
registerPtyHandlers(firstWindow as never)
const didFinishLoad = firstWindow.webContents.on.mock.calls.find(
([eventName]) => eventName === 'did-finish-load'
)?.[1] as (() => void) | undefined
expect(didFinishLoad).toBeTypeOf('function')
setLocalPtyProvider({
spawn: vi.fn(),
write: vi.fn(),
resize: vi.fn(),
kill: vi.fn(),
shutdown: vi.fn(),
onData: vi.fn(() => vi.fn()),
onExit: vi.fn(() => vi.fn()),
listProcesses: vi.fn(async () => []),
getForegroundProcess: vi.fn(async () => null)
} as never)
registerPtyHandlers(secondWindow as never)
expect(firstWindow.webContents.removeListener).toHaveBeenCalledWith(
'did-finish-load',
didFinishLoad
)
expect(
secondWindow.webContents.on.mock.calls.some(([eventName]) => eventName === 'did-finish-load')
).toBe(false)
})
it('clears PTY state even when kill reports the process is already gone', async () => {
const proc = {
onData: vi.fn(() => makeDisposable()),
+11 -3
View File
@@ -848,6 +848,7 @@ export function setPtyOwnership(id: string, connectionId: string | null): void {
let localDataUnsub: (() => void) | null = null
let localExitUnsub: (() => void) | null = null
let didFinishLoadHandler: (() => void) | null = null
let didFinishLoadWebContents: WebContents | null = null
// Why: the "Restart daemon" path needs to re-bind provider→renderer listeners
// against the freshly-created adapter after replaceDaemonProvider swaps the
@@ -861,6 +862,14 @@ export function rebindLocalProviderListeners(): void {
rebindProviderListeners?.()
}
function clearDidFinishLoadHandler(): void {
if (didFinishLoadHandler && didFinishLoadWebContents) {
didFinishLoadWebContents.removeListener('did-finish-load', didFinishLoadHandler)
}
didFinishLoadHandler = null
didFinishLoadWebContents = null
}
// Why: the "Restart daemon" flow needs to detach listeners from the current
// adapter *after* synthetic pty:exit events fan out (so the renderer receives
// them) but *before* replaceDaemonProvider swaps in the new adapter (so the
@@ -1388,11 +1397,9 @@ export function registerPtyHandlers(
// Why: only applies to LocalPtyProvider where PTYs live in the Electron main
// process and can become orphaned on page reload. Daemon-backed sessions
// survive renderer restarts by design — orphan cleanup would kill them.
clearDidFinishLoadHandler()
if (localProvider instanceof LocalPtyProvider) {
const lp = localProvider
if (didFinishLoadHandler) {
mainWindow.webContents.removeListener('did-finish-load', didFinishLoadHandler)
}
didFinishLoadHandler = () => {
const killed = lp.killOrphanedPtys(lp.advanceGeneration() - 1)
for (const { id } of killed) {
@@ -1402,6 +1409,7 @@ export function registerPtyHandlers(
runtime?.onPtyExit(id, -1)
}
}
didFinishLoadWebContents = mainWindow.webContents
mainWindow.webContents.on('did-finish-load', didFinishLoadHandler)
}