fix: clean up sleep assertion listeners (#3758)

This commit is contained in:
Neil 2026-05-30 09:04:52 -07:00 committed by GitHub
parent d0aa82864b
commit 64e0857358
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 96 additions and 14 deletions

View File

@ -90,6 +90,24 @@ describe('LinuxLidSleepAssertion', () => {
expect(child.kill).toHaveBeenCalledTimes(1)
})
it('removes child listeners when stopped intentionally', () => {
const child = new FakeSystemdInhibitProcess()
const assertion = new LinuxLidSleepAssertion({
logger: createLogger(),
platform: 'linux',
spawn: vi.fn(() => child)
})
assertion.start('status-change')
expect(child.listenerCount('error')).toBe(1)
expect(child.listenerCount('exit')).toBe(1)
assertion.stop('settings-change')
expect(child.listenerCount('error')).toBe(0)
expect(child.listenerCount('exit')).toBe(0)
})
it('does not report an intentional stop as a failed inhibitor', () => {
const logger = createLogger()
const child = new FakeSystemdInhibitProcess()
@ -153,6 +171,8 @@ describe('LinuxLidSleepAssertion', () => {
expect(onUnexpectedFailure).toHaveBeenCalledWith('linux-lid-assertion-failure')
expect(spawn).toHaveBeenCalledTimes(2)
expect(logger.warn).toHaveBeenCalledTimes(1)
expect(firstChild.listenerCount('error')).toBe(0)
expect(firstChild.listenerCount('exit')).toBe(0)
})
it('suppresses retry attempts until the shared retry gate expires', () => {

View File

@ -4,12 +4,15 @@ export const LINUX_LID_SLEEP_ASSERTION_RETRY_MS = 30_000
type Logger = Pick<Console, 'debug' | 'warn'>
type SystemdInhibitErrorListener = (error: Error & { code?: string }) => void
type SystemdInhibitExitListener = (code: number | null, signal: NodeJS.Signals | null) => void
type SystemdInhibitProcess = {
kill: () => boolean
on: {
(event: 'error', listener: (error: Error & { code?: string }) => void): void
(event: 'exit', listener: (code: number | null, signal: NodeJS.Signals | null) => void): void
}
on(event: 'error', listener: SystemdInhibitErrorListener): void
on(event: 'exit', listener: SystemdInhibitExitListener): void
off(event: 'error', listener: SystemdInhibitErrorListener): void
off(event: 'exit', listener: SystemdInhibitExitListener): void
pid?: number
}
@ -41,6 +44,7 @@ export class LinuxLidSleepAssertion {
private warnedForLastFailure = false
private readonly intentionalStops = new WeakSet<SystemdInhibitProcess>()
private readonly reportedFailures = new WeakSet<SystemdInhibitProcess>()
private readonly childCleanups = new WeakMap<SystemdInhibitProcess, () => void>()
constructor(options: LinuxLidSleepAssertionOptions = {}) {
this.logger = options.logger ?? console
@ -84,7 +88,7 @@ export class LinuxLidSleepAssertion {
}
this.child = child
child.on('error', (error) => {
const onError: SystemdInhibitErrorListener = (error) => {
this.handleChildFailure(
child,
`error:${String(error.code ?? error.message)}`,
@ -92,13 +96,19 @@ export class LinuxLidSleepAssertion {
reason,
error
)
})
child.on('exit', (code, signal) => {
}
const onExit: SystemdInhibitExitListener = (code, signal) => {
this.handleChildFailure(child, `exit:${String(code)}:${String(signal)}`, 'exit', reason, {
code,
signal
})
}
this.childCleanups.set(child, () => {
child.off('error', onError)
child.off('exit', onExit)
})
child.on('error', onError)
child.on('exit', onExit)
this.resetRetrySuppression()
this.resetFailureStreak()
}
@ -112,6 +122,7 @@ export class LinuxLidSleepAssertion {
const child = this.child
this.child = null
this.intentionalStops.add(child)
this.detachChildListeners(child)
try {
child.kill()
} catch (error) {
@ -132,6 +143,7 @@ export class LinuxLidSleepAssertion {
startReason: string,
details: unknown
): void {
this.detachChildListeners(child)
if (this.intentionalStops.has(child)) {
this.intentionalStops.delete(child)
return
@ -146,6 +158,15 @@ export class LinuxLidSleepAssertion {
this.handleFailure(failureKey, startReason, details, failureType)
}
private detachChildListeners(child: SystemdInhibitProcess): void {
const cleanup = this.childCleanups.get(child)
if (!cleanup) {
return
}
cleanup()
this.childCleanups.delete(child)
}
private handleFailure(
failureKey: string,
reason: string,

View File

@ -79,6 +79,24 @@ describe('MacosSystemSleepAssertion', () => {
expect(child.kill).toHaveBeenCalledTimes(1)
})
it('removes child listeners when stopped intentionally', () => {
const child = new FakeCaffeinateProcess()
const assertion = new MacosSystemSleepAssertion({
logger: createLogger(),
platform: 'darwin',
spawn: vi.fn(() => child)
})
assertion.start('status-change')
expect(child.listenerCount('error')).toBe(1)
expect(child.listenerCount('exit')).toBe(1)
assertion.stop('settings-change')
expect(child.listenerCount('error')).toBe(0)
expect(child.listenerCount('exit')).toBe(0)
})
it('clears the child and notifies the owner on unexpected exit', () => {
const firstChild = new FakeCaffeinateProcess()
const secondChild = new FakeCaffeinateProcess()
@ -101,6 +119,8 @@ describe('MacosSystemSleepAssertion', () => {
expect(onUnexpectedFailure).toHaveBeenCalledWith('macos-assertion-failure')
expect(spawn).toHaveBeenCalledTimes(2)
expect(firstChild.listenerCount('error')).toBe(0)
expect(firstChild.listenerCount('exit')).toBe(0)
})
it('does not report an intentional stop as unexpected', () => {

View File

@ -4,12 +4,15 @@ export const MACOS_SYSTEM_SLEEP_ASSERTION_RETRY_MS = 30_000
type Logger = Pick<Console, 'debug' | 'warn'>
type CaffeinateErrorListener = (error: Error) => void
type CaffeinateExitListener = (code: number | null, signal: NodeJS.Signals | null) => void
type CaffeinateProcess = {
kill: () => boolean
on: {
(event: 'error', listener: (error: Error) => void): void
(event: 'exit', listener: (code: number | null, signal: NodeJS.Signals | null) => void): void
}
on(event: 'error', listener: CaffeinateErrorListener): void
on(event: 'exit', listener: CaffeinateExitListener): void
off(event: 'error', listener: CaffeinateErrorListener): void
off(event: 'exit', listener: CaffeinateExitListener): void
pid?: number
}
@ -40,6 +43,7 @@ export class MacosSystemSleepAssertion {
private warnedForLastFailure = false
private readonly intentionalStops = new WeakSet<CaffeinateProcess>()
private readonly reportedFailures = new WeakSet<CaffeinateProcess>()
private readonly childCleanups = new WeakMap<CaffeinateProcess, () => void>()
constructor(options: MacosSystemSleepAssertionOptions = {}) {
this.logger = options.logger ?? console
@ -70,15 +74,21 @@ export class MacosSystemSleepAssertion {
}
this.child = child
child.on('error', (error) => {
const onError: CaffeinateErrorListener = (error) => {
this.handleChildFailure(child, `error:${String(error.message)}`, 'error', reason, error)
})
child.on('exit', (code, signal) => {
}
const onExit: CaffeinateExitListener = (code, signal) => {
this.handleChildFailure(child, `exit:${String(code)}:${String(signal)}`, 'exit', reason, {
code,
signal
})
}
this.childCleanups.set(child, () => {
child.off('error', onError)
child.off('exit', onExit)
})
child.on('error', onError)
child.on('exit', onExit)
this.resetRetrySuppression()
this.resetFailureStreak()
}
@ -92,6 +102,7 @@ export class MacosSystemSleepAssertion {
const child = this.child
this.child = null
this.intentionalStops.add(child)
this.detachChildListeners(child)
try {
child.kill()
} catch (error) {
@ -114,6 +125,7 @@ export class MacosSystemSleepAssertion {
startReason: string,
details: unknown
): void {
this.detachChildListeners(child)
if (this.intentionalStops.has(child)) {
this.intentionalStops.delete(child)
return
@ -128,6 +140,15 @@ export class MacosSystemSleepAssertion {
this.handleFailure(failureKey, startReason, details, failureType)
}
private detachChildListeners(child: CaffeinateProcess): void {
const cleanup = this.childCleanups.get(child)
if (!cleanup) {
return
}
cleanup()
this.childCleanups.delete(child)
}
private handleFailure(
failureKey: string,
reason: string,