mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-11 03:38:07 +00:00
fix(lbug): make the conn-lock non-reentrancy invariant enforced, not just documented (#2264)
A future withConnLock-wrapped helper calling another wrapped helper would await its own holder's tail and hang silently. Add an AsyncLocalStorage-based re-entry guard: withConnLock throws a clear error when invoked from within a holding fn's async context. A boolean flag can't do this — a legitimately-queued top-level caller also runs while the lock is held; only AsyncLocalStorage distinguishes a true nested call from normal contention. Tests: re-entry throws (not deadlocks); sequential and concurrent top-level calls do NOT false-fire; the lock releases after a re-entry throw. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JBJomjoTdBV2eveDVq4JMm
This commit is contained in:
parent
ed72068555
commit
f0cb368580
2 changed files with 57 additions and 3 deletions
|
|
@ -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<void> = 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<true>();
|
||||
|
||||
export const withConnLock = async <T>(fn: () => Promise<T>): Promise<T> => {
|
||||
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<void>((resolve) => {
|
||||
|
|
@ -30,7 +48,7 @@ export const withConnLock = async <T>(fn: () => Promise<T>): Promise<T> => {
|
|||
});
|
||||
await prior;
|
||||
try {
|
||||
return await fn();
|
||||
return await inCriticalSection.run(true, fn);
|
||||
} finally {
|
||||
release();
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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');
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue