fix: a bare script now exits cleanly after close() — release every process keep-alive
A consumer report on the GA pair: any minimal add/find/close script hangs forever. Root cause was FOUR ref'd keep-alives brainy never released: 1+2. The global SIGTERM/SIGINT shutdown hooks (registered once at init) were anonymous and never removed — a process.on signal listener holds a ref'd signal handle. They are now named statics, deregistered when the LAST live instance closes (Brainy.instances also stopped leaking closed instances); a later init re-registers them. 3. UnifiedCache.startFairnessMonitor() created an interval it never stored, cleared, or unref'd — unclearable by construction. Now stored + unref'd (same posture as the existing memory-pressure timer): a cache monitor must never keep the host process alive. 4. brainy.close() never called VirtualFileSystem.close(), stranding the VFS background-maintenance interval and the PathResolver maintenance interval. close() now shuts the VFS down. Proof: a bare script (init/add/close, no process.exit) exits 1 ms after close() returns; before the fix it hung until killed. 3 new lifecycle tests pin the hook semantics (once globally, survive while other instances live, removed on last close, re-register after).
This commit is contained in:
parent
ef2022ffd3
commit
c540d63b69
3 changed files with 146 additions and 11 deletions
|
|
@ -327,6 +327,15 @@ export class Brainy<T = any> implements BrainyInterface<T> {
|
||||||
private static shutdownHooksRegisteredGlobally = false
|
private static shutdownHooksRegisteredGlobally = false
|
||||||
private static instances: Brainy[] = []
|
private static instances: Brainy[] = []
|
||||||
|
|
||||||
|
/** The globally-registered shutdown listeners, kept so the LAST close() can
|
||||||
|
* deregister them. A process.on('SIGINT'/'SIGTERM') listener holds a ref'd
|
||||||
|
* signal handle that keeps the Node event loop alive — a library that never
|
||||||
|
* removes its listeners makes every bare script hang after close().
|
||||||
|
* See {@link registerShutdownHooks} / {@link deregisterShutdownHooksIfIdle}. */
|
||||||
|
private static sigtermListener?: () => void
|
||||||
|
private static sigintListener?: () => void
|
||||||
|
private static beforeExitListener?: () => void
|
||||||
|
|
||||||
/** 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
|
||||||
|
|
@ -1240,20 +1249,43 @@ export class Brainy<T = any> implements BrainyInterface<T> {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Graceful shutdown signals (registered once globally)
|
// Graceful shutdown signals (registered once globally). The listeners are
|
||||||
process.on('SIGTERM', async () => {
|
// kept as statics so the last live instance's close() can deregister them
|
||||||
|
// — the signal handles they hold are ref'd and would otherwise keep the
|
||||||
|
// process alive forever after every brain is closed.
|
||||||
|
Brainy.sigtermListener = async () => {
|
||||||
await flushOnShutdown()
|
await flushOnShutdown()
|
||||||
process.exit(0)
|
process.exit(0)
|
||||||
})
|
}
|
||||||
|
Brainy.sigintListener = async () => {
|
||||||
process.on('SIGINT', async () => {
|
|
||||||
await flushOnShutdown()
|
await flushOnShutdown()
|
||||||
process.exit(0)
|
process.exit(0)
|
||||||
})
|
}
|
||||||
|
Brainy.beforeExitListener = async () => {
|
||||||
process.on('beforeExit', async () => {
|
|
||||||
await flushOnShutdown()
|
await flushOnShutdown()
|
||||||
})
|
}
|
||||||
|
process.on('SIGTERM', Brainy.sigtermListener)
|
||||||
|
process.on('SIGINT', Brainy.sigintListener)
|
||||||
|
process.on('beforeExit', Brainy.beforeExitListener)
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Deregister the global shutdown hooks once NO live instance remains, so a
|
||||||
|
* script that closed every brain exits on its own — a library must never
|
||||||
|
* keep its host process alive. Re-initializing later re-registers them
|
||||||
|
* (the `shutdownHooksRegisteredGlobally` flag resets here).
|
||||||
|
*/
|
||||||
|
private static deregisterShutdownHooksIfIdle(): void {
|
||||||
|
if (Brainy.instances.length > 0 || !Brainy.shutdownHooksRegisteredGlobally) {
|
||||||
|
return
|
||||||
|
}
|
||||||
|
if (Brainy.sigtermListener) process.off('SIGTERM', Brainy.sigtermListener)
|
||||||
|
if (Brainy.sigintListener) process.off('SIGINT', Brainy.sigintListener)
|
||||||
|
if (Brainy.beforeExitListener) process.off('beforeExit', Brainy.beforeExitListener)
|
||||||
|
Brainy.sigtermListener = undefined
|
||||||
|
Brainy.sigintListener = undefined
|
||||||
|
Brainy.beforeExitListener = undefined
|
||||||
|
Brainy.shutdownHooksRegisteredGlobally = false
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|
@ -14262,10 +14294,26 @@ export class Brainy<T = any> implements BrainyInterface<T> {
|
||||||
await this.storage.releaseWriterLock()
|
await this.storage.releaseWriterLock()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Shut down the VFS: stops its background maintenance interval and the
|
||||||
|
// PathResolver's — both are ref'd timers that would keep the process
|
||||||
|
// alive after the last brain closes (consumer-reported hang).
|
||||||
|
if (this._vfs) {
|
||||||
|
await this._vfs.close()
|
||||||
|
}
|
||||||
|
|
||||||
this.initialized = false
|
this.initialized = false
|
||||||
// close() is terminal: block lazy re-initialization on any subsequent
|
// close() is terminal: block lazy re-initialization on any subsequent
|
||||||
// operation (ensureInitialized() throws once this is set).
|
// operation (ensureInitialized() throws once this is set).
|
||||||
this.closed = true
|
this.closed = true
|
||||||
|
|
||||||
|
// Drop this instance from the global registry, and when it was the last
|
||||||
|
// one, deregister the global shutdown hooks — their ref'd signal handles
|
||||||
|
// would otherwise keep the process alive after every brain is closed.
|
||||||
|
const instanceIndex = Brainy.instances.indexOf(this)
|
||||||
|
if (instanceIndex !== -1) {
|
||||||
|
Brainy.instances.splice(instanceIndex, 1)
|
||||||
|
}
|
||||||
|
Brainy.deregisterShutdownHooksIfIdle()
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -79,6 +79,7 @@ export class UnifiedCache implements CacheProvider {
|
||||||
private readonly memoryInfo: MemoryInfo
|
private readonly memoryInfo: MemoryInfo
|
||||||
private readonly allocationStrategy: CacheAllocationStrategy
|
private readonly allocationStrategy: CacheAllocationStrategy
|
||||||
private memoryPressureCheckTimer: NodeJS.Timeout | null = null
|
private memoryPressureCheckTimer: NodeJS.Timeout | null = null
|
||||||
|
private fairnessMonitorTimer: NodeJS.Timeout | null = null
|
||||||
private lastMemoryWarning = 0
|
private lastMemoryWarning = 0
|
||||||
|
|
||||||
constructor(config: UnifiedCacheConfig = {}) {
|
constructor(config: UnifiedCacheConfig = {}) {
|
||||||
|
|
@ -313,12 +314,19 @@ export class UnifiedCache implements CacheProvider {
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Fairness monitoring - prevent one type from hogging cache
|
* Fairness monitoring - prevent one type from hogging cache.
|
||||||
|
*
|
||||||
|
* The interval is unref'd: a background cache monitor must never keep the
|
||||||
|
* host process alive (this exact timer made bare scripts hang after
|
||||||
|
* `brain.close()` — the loop could not drain while it stayed ref'd).
|
||||||
*/
|
*/
|
||||||
private startFairnessMonitor(): void {
|
private startFairnessMonitor(): void {
|
||||||
setInterval(() => {
|
this.fairnessMonitorTimer = setInterval(() => {
|
||||||
this.checkFairness()
|
this.checkFairness()
|
||||||
}, this.config.fairnessCheckInterval!)
|
}, this.config.fairnessCheckInterval!)
|
||||||
|
if (this.fairnessMonitorTimer.unref) {
|
||||||
|
this.fairnessMonitorTimer.unref()
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
private checkFairness(): void {
|
private checkFairness(): void {
|
||||||
|
|
|
||||||
79
tests/unit/shutdown-hooks-lifecycle.test.ts
Normal file
79
tests/unit/shutdown-hooks-lifecycle.test.ts
Normal file
|
|
@ -0,0 +1,79 @@
|
||||||
|
/**
|
||||||
|
* Global shutdown-hook lifecycle — a library must never keep its host process
|
||||||
|
* alive. init() registers process-level SIGTERM/SIGINT/beforeExit listeners
|
||||||
|
* (once, globally) whose signal handles are ref'd by Node; if they outlive the
|
||||||
|
* last brain, a bare script hangs forever after close() (consumer-reported on
|
||||||
|
* the GA pair). The contract pinned here:
|
||||||
|
*
|
||||||
|
* - first init() → exactly one listener added per signal
|
||||||
|
* - more inits → no additional listeners (registered once)
|
||||||
|
* - close() → listeners survive while OTHER instances remain live
|
||||||
|
* - last close() → listeners removed, counts back to baseline
|
||||||
|
* - re-init → hooks re-register (the once-flag resets)
|
||||||
|
*/
|
||||||
|
import { describe, it, expect } from 'vitest'
|
||||||
|
import { Brainy, NounType } from '../../src/index.js'
|
||||||
|
|
||||||
|
const SIGNALS = ['SIGTERM', 'SIGINT', 'beforeExit'] as const
|
||||||
|
|
||||||
|
function listenerCounts(): Record<string, number> {
|
||||||
|
return Object.fromEntries(SIGNALS.map((s) => [s, process.listeners(s as any).length]))
|
||||||
|
}
|
||||||
|
|
||||||
|
function makeBrain(): any {
|
||||||
|
return new Brainy({ requireSubtype: false, storage: { type: 'memory' }, plugins: [], silent: true })
|
||||||
|
}
|
||||||
|
|
||||||
|
describe('Shutdown-hook lifecycle (process must exit after the last close)', () => {
|
||||||
|
it('registers each hook once on first init and removes ALL of them on the last close', async () => {
|
||||||
|
const baseline = listenerCounts()
|
||||||
|
|
||||||
|
const brain = makeBrain()
|
||||||
|
await brain.init()
|
||||||
|
for (const s of SIGNALS) {
|
||||||
|
expect(process.listeners(s as any).length).toBe(baseline[s] + 1) // exactly one each
|
||||||
|
}
|
||||||
|
|
||||||
|
await brain.close()
|
||||||
|
expect(listenerCounts()).toEqual(baseline) // gone — the loop can drain
|
||||||
|
})
|
||||||
|
|
||||||
|
it('keeps the hooks while any other instance is live; only the LAST close deregisters', async () => {
|
||||||
|
const baseline = listenerCounts()
|
||||||
|
|
||||||
|
const first = makeBrain()
|
||||||
|
const second = makeBrain()
|
||||||
|
await first.init()
|
||||||
|
await second.init()
|
||||||
|
for (const s of SIGNALS) {
|
||||||
|
expect(process.listeners(s as any).length).toBe(baseline[s] + 1) // once globally, not per instance
|
||||||
|
}
|
||||||
|
|
||||||
|
await first.close()
|
||||||
|
for (const s of SIGNALS) {
|
||||||
|
expect(process.listeners(s as any).length).toBe(baseline[s] + 1) // second is still live
|
||||||
|
}
|
||||||
|
|
||||||
|
await second.close()
|
||||||
|
expect(listenerCounts()).toEqual(baseline)
|
||||||
|
})
|
||||||
|
|
||||||
|
it('re-registers after a full close/re-init cycle (the once-flag resets)', async () => {
|
||||||
|
const baseline = listenerCounts()
|
||||||
|
|
||||||
|
const first = makeBrain()
|
||||||
|
await first.init()
|
||||||
|
await first.close()
|
||||||
|
expect(listenerCounts()).toEqual(baseline)
|
||||||
|
|
||||||
|
const second = makeBrain()
|
||||||
|
await second.init()
|
||||||
|
const id = await second.add({ data: 'still fully functional', type: NounType.Concept })
|
||||||
|
expect(id).toBeTruthy()
|
||||||
|
for (const s of SIGNALS) {
|
||||||
|
expect(process.listeners(s as any).length).toBe(baseline[s] + 1)
|
||||||
|
}
|
||||||
|
await second.close()
|
||||||
|
expect(listenerCounts()).toEqual(baseline)
|
||||||
|
})
|
||||||
|
})
|
||||||
Loading…
Add table
Add a link
Reference in a new issue