fix(shutdown): hold the signal listener until the exit decision is made
Some checks failed
CI / Node 24 (push) Successful in 12m16s
CI / Node 22 (push) Successful in 12m31s
CI / Bun (latest) (push) Successful in 12m32s
CI / Integration + conformance (Node 22) (push) Failing after 16m4s
Some checks failed
CI / Node 24 (push) Successful in 12m16s
CI / Node 22 (push) Successful in 12m31s
CI / Bun (latest) (push) Successful in 12m32s
CI / Integration + conformance (Node 22) (push) Failing after 16m4s
Closing the last live instance inside the SIGTERM/SIGINT handler calls close() -> deregisterShutdownHooksIfIdle(), which removes Brainy's own signal listener from process synchronously, before that same handler invocation has reached the point where it decides whether to exit. That opens a window with no registered listener for the signal at all: a second/concurrent delivery of the same signal during that window falls through to Node's default disposition and kills the process outright, after the clean shutdown already finished, so the process reports a signal kill instead of the 0 clause (a) and (b) of shutdown-single-owner.test.ts pin — intermittent under load, which is why it only ever showed up on the box. Add a static flag that stays true for the whole closeOnShutdown() run and makes deregisterShutdownHooksIfIdle() defer rather than remove the listener while that run is still deciding; closeOnShutdown()'s own finally re-runs the deregistration check once it is actually done, so the listener never leaks past its use.
This commit is contained in:
parent
aac853d3e8
commit
a2ea21b330
1 changed files with 91 additions and 43 deletions
|
|
@ -544,6 +544,23 @@ export class Brainy<T = any> implements BrainyInterface<T> {
|
||||||
* and narrate forever. Reset by {@link deregisterShutdownHooksIfIdle}. */
|
* and narrate forever. Reset by {@link deregisterShutdownHooksIfIdle}. */
|
||||||
private static beforeExitNarrated = false
|
private static beforeExitNarrated = false
|
||||||
|
|
||||||
|
/** True for the entire duration of ONE `closeOnShutdown()` run (the
|
||||||
|
* signal-path handler in {@link registerShutdownHooks}) — from before it
|
||||||
|
* starts closing instances until after it has decided whether to exit.
|
||||||
|
* THE RACE THIS CLOSES: closing the LAST live instance calls
|
||||||
|
* `close()` → `deregisterShutdownHooksIfIdle()` synchronously, which
|
||||||
|
* removes `Brainy.sigtermListener` from `process` — while `closeOnShutdown`
|
||||||
|
* (that very listener's OWN still-running invocation) hasn't yet reached
|
||||||
|
* `exitIfSoleShutdownOwner()`'s `process.exit(0)`. In that window Node has
|
||||||
|
* NO registered SIGTERM listener, so a second/concurrent delivery of the
|
||||||
|
* same signal (a raced re-send, common on a loaded host) falls through to
|
||||||
|
* Node's default disposition and kills the process outright — the
|
||||||
|
* clean-shutdown work already finished, but the process never reports the
|
||||||
|
* 0 it earned. `deregisterShutdownHooksIfIdle()` checks this flag and
|
||||||
|
* defers; `closeOnShutdown()`'s `finally` re-runs the deregistration check
|
||||||
|
* once it is done, so the listener never actually leaks past its use. */
|
||||||
|
private static shutdownSignalHandlerActive = false
|
||||||
|
|
||||||
/** Poll cadence (ms) for the migration LOCK when a provider exposes no
|
/** Poll cadence (ms) for the migration LOCK when a provider exposes no
|
||||||
* event-driven `whenMigrationComplete()` signal. See {@link awaitMigrationLock}. */
|
* event-driven `whenMigrationComplete()` signal. See {@link awaitMigrationLock}. */
|
||||||
private static readonly MIGRATION_POLL_INTERVAL_MS = 250
|
private static readonly MIGRATION_POLL_INTERVAL_MS = 250
|
||||||
|
|
@ -2196,13 +2213,29 @@ export class Brainy<T = any> implements BrainyInterface<T> {
|
||||||
*/
|
*/
|
||||||
const closeOnShutdown = async () => {
|
const closeOnShutdown = async () => {
|
||||||
console.log('Shutdown signal received - flushing pending data...')
|
console.log('Shutdown signal received - flushing pending data...')
|
||||||
// DEFER ONE MACROTASK. A host application registers its own listener on
|
// HOLD THE LISTENER FOR THE WHOLE RUN. Closing the LAST live instance
|
||||||
// the same signal, and Node runs listeners in registration order — ours
|
// below calls close() → deregisterShutdownHooksIfIdle(), which removes
|
||||||
// is usually first, because the brain was opened before the host wired
|
// Brainy's own SIGTERM/SIGINT listeners from `process` — synchronously,
|
||||||
// its shutdown. Yielding once lets every other listener for this signal
|
// before THIS invocation has reached exitIfSoleShutdownOwner()'s
|
||||||
// run its synchronous prologue, so a host that calls close() gets to be
|
// process.exit(0). Left alone, that opens a window with no registered
|
||||||
// the owner. It is only a courtesy, never the safety: close()'s own
|
// listener for the signal at all, so a second/concurrent delivery of
|
||||||
// single-flight gate is what makes a lost race harmless.
|
// the same signal (a raced re-send — not rare on a loaded host) falls
|
||||||
|
// through to Node's default disposition and kills the process outright
|
||||||
|
// AFTER the clean-shutdown work already finished, reporting a signal
|
||||||
|
// kill instead of the 0 the shutdown earned. Setting this flag makes
|
||||||
|
// deregisterShutdownHooksIfIdle() defer; the `finally` below re-checks
|
||||||
|
// it once this run is fully done — closeOnShutdown, not a nested
|
||||||
|
// close(), owns exactly when the listener actually comes off.
|
||||||
|
Brainy.shutdownSignalHandlerActive = true
|
||||||
|
try {
|
||||||
|
// DEFER ONE MACROTASK. A host application registers its own listener
|
||||||
|
// on the same signal, and Node runs listeners in registration order —
|
||||||
|
// ours is usually first, because the brain was opened before the
|
||||||
|
// host wired its shutdown. Yielding once lets every other listener
|
||||||
|
// for this signal run its synchronous prologue, so a host that calls
|
||||||
|
// close() gets to be the owner. It is only a courtesy, never the
|
||||||
|
// safety: close()'s own single-flight gate is what makes a lost race
|
||||||
|
// harmless.
|
||||||
await new Promise<void>((resolve) => setImmediate(resolve))
|
await new Promise<void>((resolve) => setImmediate(resolve))
|
||||||
|
|
||||||
let closedCount = 0
|
let closedCount = 0
|
||||||
|
|
@ -2242,6 +2275,13 @@ export class Brainy<T = any> implements BrainyInterface<T> {
|
||||||
`their writer locks were released, but their next open will run crash recovery.`
|
`their writer locks were released, but their next open will run crash recovery.`
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
} finally {
|
||||||
|
// Release the hold and run the deferred check ourselves — the last
|
||||||
|
// close() above may have found the flag set and skipped its own
|
||||||
|
// deregistration, so nobody else will do this if we don't.
|
||||||
|
Brainy.shutdownSignalHandlerActive = false
|
||||||
|
Brainy.deregisterShutdownHooksIfIdle()
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|
@ -2404,9 +2444,17 @@ export class Brainy<T = any> implements BrainyInterface<T> {
|
||||||
* script that closed every brain exits on its own — a library must never
|
* script that closed every brain exits on its own — a library must never
|
||||||
* keep its host process alive. Re-initializing later re-registers them
|
* keep its host process alive. Re-initializing later re-registers them
|
||||||
* (the `shutdownHooksRegisteredGlobally` flag resets here).
|
* (the `shutdownHooksRegisteredGlobally` flag resets here).
|
||||||
|
*
|
||||||
|
* Deferred (not skipped — {@link closeOnShutdown}'s `finally` always
|
||||||
|
* re-checks) while a signal-path shutdown is actively running: that
|
||||||
|
* handler's OWN still-in-flight invocation is `Brainy.sigtermListener`, and
|
||||||
|
* removing it out from under itself — which closing the LAST instance here
|
||||||
|
* would otherwise do, synchronously, mid-run — would leave `process` with
|
||||||
|
* no listener for the signal for the remainder of that run. See
|
||||||
|
* {@link shutdownSignalHandlerActive}'s doc for the exact race this closes.
|
||||||
*/
|
*/
|
||||||
private static deregisterShutdownHooksIfIdle(): void {
|
private static deregisterShutdownHooksIfIdle(): void {
|
||||||
if (Brainy.instances.length > 0 || !Brainy.shutdownHooksRegisteredGlobally) {
|
if (Brainy.instances.length > 0 || !Brainy.shutdownHooksRegisteredGlobally || Brainy.shutdownSignalHandlerActive) {
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
if (Brainy.sigtermListener) process.off('SIGTERM', Brainy.sigtermListener)
|
if (Brainy.sigtermListener) process.off('SIGTERM', Brainy.sigtermListener)
|
||||||
|
|
|
||||||
Reference in a new issue