Skip to content

Commit 2d40307

Browse files
committed
add tests and fixed as review
1 parent 52783d8 commit 2d40307

3 files changed

Lines changed: 122 additions & 0 deletions

File tree

graphql/server/src/diagnostics/debug-memory-snapshot.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@ export const getDebugMemorySnapshot = (): DebugMemorySnapshot => {
9999
size: svcCache.size,
100100
max: svcCache.max,
101101
ttlMs: SVC_CACHE_TTL_MS,
102+
// Note: with updateAgeOnGet: true, this is "time since last access" not "time since creation"
102103
oldestKeyAgeMs: (() => {
103104
let minRemaining = Infinity;
104105
for (const key of svcCache.keys()) {
Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
// Guards against the pg-cache close() resource leak fixed in feat/observability.
2+
//
3+
// Previously, close() reset this.closed = false after shutdown, allowing
4+
// set() to silently accept new pools that were never cleaned up. The module-
5+
// level closePromise also reset to null, enabling double-shutdown.
6+
//
7+
// These tests lock the fix: close() is final, set() rejects, and repeated
8+
// close() calls are idempotent. See pg-cache-close-leak.md for full details.
9+
10+
import pg from 'pg';
11+
import { PgPoolCacheManager } from '../lru';
12+
13+
// Minimal mock — we only need pool.end() and pool.ended
14+
const createMockPool = (): pg.Pool => {
15+
let ended = false;
16+
return {
17+
get ended() { return ended; },
18+
end: jest.fn(async () => { ended = true; }),
19+
} as unknown as pg.Pool;
20+
};
21+
22+
describe('PgPoolCacheManager', () => {
23+
let cache: PgPoolCacheManager;
24+
25+
beforeEach(() => {
26+
cache = new PgPoolCacheManager();
27+
});
28+
29+
afterEach(async () => {
30+
// Ensure all pools are cleaned up even if a test fails mid-way
31+
try { await cache.close(); } catch { /* already closed */ }
32+
});
33+
34+
it('stores and retrieves a pool', () => {
35+
const pool = createMockPool();
36+
cache.set('key1', pool);
37+
expect(cache.get('key1')).toBe(pool);
38+
expect(cache.has('key1')).toBe(true);
39+
});
40+
41+
it('returns undefined for missing keys', () => {
42+
expect(cache.get('missing')).toBeUndefined();
43+
expect(cache.has('missing')).toBe(false);
44+
});
45+
46+
describe('close() lifecycle', () => {
47+
it('set() after close() throws', async () => {
48+
const pool1 = createMockPool();
49+
cache.set('key1', pool1);
50+
51+
await cache.close();
52+
53+
const pool2 = createMockPool();
54+
expect(() => cache.set('key2', pool2)).toThrow(
55+
'Cannot add to cache after it has been closed (key: key2)',
56+
);
57+
});
58+
59+
it('get() after close() returns undefined with warning', async () => {
60+
const pool = createMockPool();
61+
cache.set('key1', pool);
62+
63+
await cache.close();
64+
65+
expect(cache.get('key1')).toBeUndefined();
66+
});
67+
68+
it('double close() is idempotent', async () => {
69+
const pool = createMockPool();
70+
cache.set('key1', pool);
71+
72+
await cache.close();
73+
await cache.close(); // should not throw
74+
75+
expect(pool.end).toHaveBeenCalledTimes(1);
76+
});
77+
78+
it('close() disposes all pools', async () => {
79+
const pool1 = createMockPool();
80+
const pool2 = createMockPool();
81+
cache.set('key1', pool1);
82+
cache.set('key2', pool2);
83+
84+
await cache.close();
85+
86+
expect(pool1.end).toHaveBeenCalledTimes(1);
87+
expect(pool2.end).toHaveBeenCalledTimes(1);
88+
});
89+
});
90+
91+
describe('cleanup callbacks', () => {
92+
it('fires callback on close()', async () => {
93+
const pool = createMockPool();
94+
cache.set('key1', pool);
95+
96+
const callback = jest.fn();
97+
cache.registerCleanupCallback(callback);
98+
99+
await cache.close();
100+
101+
expect(callback).toHaveBeenCalledWith('key1');
102+
});
103+
104+
it('unregister prevents callback from firing', async () => {
105+
const pool = createMockPool();
106+
cache.set('key1', pool);
107+
108+
const callback = jest.fn();
109+
const unregister = cache.registerCleanupCallback(callback);
110+
unregister();
111+
112+
await cache.close();
113+
114+
expect(callback).not.toHaveBeenCalled();
115+
});
116+
});
117+
});

postgres/pg-cache/src/lru.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,10 @@ export class PgPoolCacheManager {
7070
}
7171

7272
get(key: PgPoolKey): pg.Pool | undefined {
73+
if (this.closed) {
74+
log.warn(`Cache is closed, ignoring get(${key})`);
75+
return undefined;
76+
}
7377
return this.pgCache.get(key)?.pool;
7478
}
7579

0 commit comments

Comments
 (0)