Skip to content

Commit deb98ca

Browse files
committed
Avoid repeated connect lease deadlock
1 parent 68ce815 commit deb98ca

2 files changed

Lines changed: 61 additions & 15 deletions

File tree

src/utils/connection.test.ts

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -644,6 +644,47 @@ describe('DatabaseConnection lifecycle', () => {
644644
}
645645
});
646646

647+
it('reuses an active same-pool connection without waiting on its lease', async () => {
648+
const release = vi.fn();
649+
const client = {
650+
query: vi.fn().mockResolvedValue({ rows: [] }),
651+
release
652+
};
653+
const pool = {
654+
connect: vi.fn().mockResolvedValue(client),
655+
on: vi.fn(),
656+
end: vi.fn().mockResolvedValue(undefined)
657+
};
658+
const Pool = vi.fn(() => pool);
659+
660+
vi.resetModules();
661+
vi.doMock('pg', () => ({ default: { Pool } }));
662+
vi.doMock('pg-monitor', () => ({
663+
default: {
664+
attach: vi.fn(),
665+
setTheme: vi.fn()
666+
}
667+
}));
668+
669+
try {
670+
const { DatabaseConnection: FreshDatabaseConnection } = await import('./connection');
671+
const db = FreshDatabaseConnection.getInstance();
672+
673+
await db.connect('postgresql://same');
674+
await db.connect('postgresql://same');
675+
676+
expect(Pool).toHaveBeenCalledTimes(1);
677+
expect(pool.connect).toHaveBeenCalledTimes(1);
678+
expect(client.query).toHaveBeenCalledTimes(1);
679+
680+
await FreshDatabaseConnection.cleanupPools();
681+
} finally {
682+
vi.doUnmock('pg');
683+
vi.doUnmock('pg-monitor');
684+
vi.resetModules();
685+
}
686+
});
687+
647688
it('releases the connection lease when connect validation fails', async () => {
648689
const release = vi.fn();
649690
const client = {

src/utils/connection.ts

Lines changed: 20 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -263,25 +263,30 @@ export class DatabaseConnection {
263263
* Connect to a PostgreSQL database
264264
*/
265265
public async connect(connectionString?: string, options: ConnectionOptions = {}): Promise<void> {
266-
await this.acquireConnectionLease();
267-
let connectionStateChanged = false;
266+
// Use environment variable only when no explicit connection string was provided.
267+
const connString = connectionString !== undefined ? connectionString : process.env.POSTGRES_CONNECTION_STRING;
268+
const effectiveOptions = {
269+
...getDefaultConnectionOptions(),
270+
...options
271+
};
272+
273+
if (connString === undefined || connString.trim() === '') {
274+
throw new Error('No non-empty connection string provided and POSTGRES_CONNECTION_STRING environment variable is not set');
275+
}
268276

269-
try {
270-
// Use environment variable only when no explicit connection string was provided.
271-
const connString = connectionString !== undefined ? connectionString : process.env.POSTGRES_CONNECTION_STRING;
272-
const effectiveOptions = {
273-
...getDefaultConnectionOptions(),
274-
...options
275-
};
277+
assertConnectionOptions(effectiveOptions);
278+
const poolCacheKey = buildPoolCacheKey(connString, effectiveOptions);
276279

277-
if (connString === undefined || connString.trim() === '') {
278-
throw new Error('No non-empty connection string provided and POSTGRES_CONNECTION_STRING environment variable is not set');
279-
}
280+
// If already connected to this database, reuse the connection without waiting on our own active lease.
281+
if (this.pool && this.poolCacheKey === poolCacheKey) {
282+
return;
283+
}
280284

281-
assertConnectionOptions(effectiveOptions);
282-
const poolCacheKey = buildPoolCacheKey(connString, effectiveOptions);
285+
await this.acquireConnectionLease();
286+
let connectionStateChanged = false;
283287

284-
// If already connected to this database, reuse the connection
288+
try {
289+
// Another queued connect may have established the requested pool while we waited.
285290
if (this.pool && this.poolCacheKey === poolCacheKey) {
286291
return;
287292
}

0 commit comments

Comments
 (0)