fix: detach macos resolver probe listeners

Detach scutil stream and child process listeners when the macOS resolver probe settles, with listener-count regression coverage. Risk: low.
This commit is contained in:
Neil
2026-05-31 01:48:30 -07:00
committed by GitHub
parent 45afdfc3c2
commit b4dba9af73
3 changed files with 111 additions and 11 deletions
@@ -0,0 +1,43 @@
# Memory Leak Audit Pass 4
Started: 2026-05-31 PDT
Objective: continue the memory leak audit on current `origin/main` after pass 3
and submit one PR per confirmed issue.
## Delta Inventory
- 2026-05-31: Rebases completed on current `origin/main`
(`7d91c5c5d3`, `perf: coalesce startup history gc`).
- 2026-05-31: Counted 14 changed code files since pass 3
(`41cbdb1d1d65f9c9538a2347684e1f4ddb3f06ec`).
- 2026-05-31: Re-ran heuristics for DOM listeners, timers, animation frames,
observers, EventEmitter subscriptions, runtime subscriptions, child process
streams, and module-scope `Map`/`Set` caches.
- 2026-05-31: Manually followed up delta hits in daemon sockets, macOS resolver
health probing, workspace port scanning, onboarding timers, terminal history
GC, browser webview listeners, terminal pane listeners, and diff comment view
zones.
## Finding
- `src/main/network/macos-system-resolver-health.ts`: the macOS resolver probe
resolved on timeout after killing `scutil`, but kept stdout/stderr data
listeners and child `error`/`close` listeners attached until the process
eventually closed. If `scutil` was slow or stuck after `SIGTERM`, those
listeners retained the request closure after the daemon RPC had already
settled. Fixed by using named listeners and detaching them in the shared
settlement path for both timeout and normal child close. Risk: low.
## Validation
- `pnpm exec vitest run --config config/vitest.config.ts src/main/network/macos-system-resolver-health.test.ts src/main/daemon/daemon-server.test.ts`
- `pnpm exec oxlint src/main/network/macos-system-resolver-health.ts src/main/network/macos-system-resolver-health.test.ts src/main/daemon/daemon-server.ts src/main/daemon/daemon-server.test.ts docs/reference/memory-leak-audit-pass-4.md`
- `pnpm run typecheck:node`
- `git diff --check`
## Remaining Work
- Continue the current-state repository audit. This pass covered the post-pass-3
delta and one confirmed leak; it does not prove the full repository is
entirely leak-free.
@@ -109,4 +109,51 @@ resolver #1
await expect(healthPromise).resolves.toBe('unknown')
expect(child.kill).toHaveBeenCalledTimes(1)
})
it('removes scutil listeners when the timeout settles before child close', async () => {
vi.useFakeTimers()
mockPlatform('darwin')
const child = createMockScutilProcess()
vi.mocked(spawn).mockReturnValue(child)
const healthPromise = readCurrentProcessMacSystemResolverHealth()
expect(child.stdout.listenerCount('data')).toBe(1)
expect(child.stderr.listenerCount('data')).toBe(1)
expect(child.listenerCount('error')).toBe(1)
expect(child.listenerCount('close')).toBe(1)
await vi.advanceTimersByTimeAsync(1_500)
await expect(healthPromise).resolves.toBe('unknown')
expect(child.stdout.listenerCount('data')).toBe(0)
expect(child.stderr.listenerCount('data')).toBe(0)
expect(child.listenerCount('error')).toBe(0)
expect(child.listenerCount('close')).toBe(0)
})
it('removes scutil listeners when the child closes normally', async () => {
mockPlatform('darwin')
const child = createMockScutilProcess()
vi.mocked(spawn).mockReturnValue(child)
const healthPromise = readCurrentProcessMacSystemResolverHealth()
child.stdout.emit(
'data',
`
DNS configuration
resolver #1
nameserver[0] : 1.1.1.1
`
)
child.emit('close', 0)
await expect(healthPromise).resolves.toBe('healthy')
expect(child.stdout.listenerCount('data')).toBe(0)
expect(child.stderr.listenerCount('data')).toBe(0)
expect(child.listenerCount('error')).toBe(0)
expect(child.listenerCount('close')).toBe(0)
})
})
@@ -25,18 +25,32 @@ export async function readCurrentProcessMacSystemResolverHealth(): Promise<Syste
let stdout = ''
let stderr = ''
let settled = false
let timer: ReturnType<typeof setTimeout> | null = null
const child = spawn('/usr/sbin/scutil', ['--dns'], {
stdio: ['ignore', 'pipe', 'pipe']
})
const onStdoutData = (chunk: string): void => {
stdout += chunk
}
const onStderrData = (chunk: string): void => {
stderr += chunk
}
const finish = (): void => {
if (settled) {
return
}
settled = true
clearTimeout(timer)
if (timer !== null) {
clearTimeout(timer)
timer = null
}
child.stdout.off('data', onStdoutData)
child.stderr.off('data', onStderrData)
child.off('error', finish)
child.off('close', finish)
resolve(classifyMacSystemResolverHealth(`${stdout}\n${stderr}`))
}
const child = spawn('/usr/sbin/scutil', ['--dns'], {
stdio: ['ignore', 'pipe', 'pipe']
})
const timer = setTimeout(() => {
timer = setTimeout(() => {
child.kill()
// Why: this runs inside the daemon request path, so the timeout must
// cap the RPC even if scutil is slow to exit after SIGTERM.
@@ -44,12 +58,8 @@ export async function readCurrentProcessMacSystemResolverHealth(): Promise<Syste
}, MAC_RESOLVER_CHECK_TIMEOUT_MS)
child.stdout.setEncoding('utf8')
child.stderr.setEncoding('utf8')
child.stdout.on('data', (chunk: string) => {
stdout += chunk
})
child.stderr.on('data', (chunk: string) => {
stderr += chunk
})
child.stdout.on('data', onStdoutData)
child.stderr.on('data', onStderrData)
child.on('error', finish)
child.on('close', finish)
})