diff --git a/gitnexus/src/core/lbug/conn-lock.ts b/gitnexus/src/core/lbug/conn-lock.ts index bb10d8113..3a92bb8cc 100644 --- a/gitnexus/src/core/lbug/conn-lock.ts +++ b/gitnexus/src/core/lbug/conn-lock.ts @@ -17,12 +17,30 @@ * * Implementation: a promise chain. Each caller installs a fresh unresolved tail, * awaits the previous holder's tail, runs, then releases its own in `finally` - * (so a thrown op never wedges the connection). FIFO and re-entrancy-unsafe by - * design — a wrapped helper MUST NOT call another wrapped helper. + * (so a thrown op never wedges the connection). FIFO and non-reentrant: a wrapped + * helper MUST NOT call another wrapped helper — the inner call would await its own + * holder's tail and deadlock. The re-entry guard below catches this and throws + * instead of hanging. A boolean flag can't do this: a legitimately-queued + * top-level caller also runs while the lock is held, so only AsyncLocalStorage — + * which marks the *async context* of the running `fn` — distinguishes a true + * nested call from normal contention. */ +import { AsyncLocalStorage } from 'node:async_hooks'; + let tail: Promise = Promise.resolve(); +// Set (to `true`) only inside a holding `fn`'s async context. A withConnLock call +// that observes it set is a nested/re-entrant call from within a critical section. +const inCriticalSection = new AsyncLocalStorage(); + export const withConnLock = async (fn: () => Promise): Promise => { + if (inCriticalSection.getStore()) { + throw new Error( + 'conn-lock re-entry: a withConnLock-wrapped helper called another wrapped ' + + 'helper, which would deadlock the single LadybugDB connection. Run the inner ' + + 'work outside the lock, or inline it. See src/core/lbug/conn-lock.ts.', + ); + } const prior = tail; let release!: () => void; tail = new Promise((resolve) => { @@ -30,7 +48,7 @@ export const withConnLock = async (fn: () => Promise): Promise => { }); await prior; try { - return await fn(); + return await inCriticalSection.run(true, fn); } finally { release(); } diff --git a/gitnexus/test/unit/conn-lock.test.ts b/gitnexus/test/unit/conn-lock.test.ts index cbce6ac0c..699160b94 100644 --- a/gitnexus/test/unit/conn-lock.test.ts +++ b/gitnexus/test/unit/conn-lock.test.ts @@ -71,3 +71,39 @@ describe('withConnLock — connection serialization', () => { await expect(withConnLock(async () => 42)).resolves.toBe(42); }); }); + +describe('withConnLock — re-entry guard', () => { + it('throws on a nested (wrapped-in-wrapped) call instead of deadlocking', async () => { + await expect(withConnLock(async () => withConnLock(async () => 'inner'))).rejects.toThrow( + /re-entry/, + ); + }); + + it('does NOT false-fire on sequential (non-nested) calls', async () => { + // Mirrors getLbugStats: many withConnLock calls in a loop, each awaited to + // completion before the next — distinct async contexts, never nested. + const results: number[] = []; + for (let i = 0; i < 5; i++) { + results.push(await withConnLock(async () => i)); + } + expect(results).toEqual([0, 1, 2, 3, 4]); + }); + + it('does NOT false-fire on concurrent top-level (queued) callers', async () => { + // Legitimate contention: B and C call while A holds the lock. They are + // separate async contexts (not nested in A's fn), so they queue, not throw. + const out = await Promise.all([ + withConnLock(async () => 'a'), + withConnLock(async () => 'b'), + withConnLock(async () => 'c'), + ]); + expect(out).toEqual(['a', 'b', 'c']); + }); + + it('releases the lock after a re-entry throw so later callers proceed', async () => { + await expect(withConnLock(async () => withConnLock(async () => 'inner'))).rejects.toThrow( + /re-entry/, + ); + await expect(withConnLock(async () => 'ok')).resolves.toBe('ok'); + }); +});