From 84ee38eb0c9f6b7298623ebe2ebaff0ad6d61051 Mon Sep 17 00:00:00 2001 From: Sudarshan Soma <48428602+sudarshan12s@users.noreply.github.com> Date: Tue, 30 Jun 2026 15:06:15 +0530 Subject: [PATCH] fix(core): Guard connection pool idle checks during cleanup (#31059) --- .../nodes/Oracle/Sql/transport/index.ts | 1 + .../__tests__/connection-pool-manager.test.ts | 39 +++++++++++++++++++ .../utils/connection-pool-manager.ts | 16 +++++++- 3 files changed, 55 insertions(+), 1 deletion(-) diff --git a/packages/nodes-base/nodes/Oracle/Sql/transport/index.ts b/packages/nodes-base/nodes/Oracle/Sql/transport/index.ts index 24584ead9b1..bd9eed33e02 100644 --- a/packages/nodes-base/nodes/Oracle/Sql/transport/index.ts +++ b/packages/nodes-base/nodes/Oracle/Sql/transport/index.ts @@ -58,6 +58,7 @@ export async function configureOracleDB( nodeType: 'oracledb', nodeVersion: String(options.nodeVersion ?? '1'), fallBackHandler, + isIdle: (pool) => pool.connectionsInUse === 0, wasUsed: (pool) => { if (pool) { this.logger.debug(`DB pool reused, open connections: ${pool.connectionsOpen}`); diff --git a/packages/nodes-base/utils/__tests__/connection-pool-manager.test.ts b/packages/nodes-base/utils/__tests__/connection-pool-manager.test.ts index 53db7edfc23..ad6bd3452f0 100644 --- a/packages/nodes-base/utils/__tests__/connection-pool-manager.test.ts +++ b/packages/nodes-base/utils/__tests__/connection-pool-manager.test.ts @@ -190,6 +190,45 @@ describe('getConnection', () => { expect(abortController.signal.aborted).toBe(true); }); + test('postpones stale cleanup while pool is not idle', async () => { + // ARRANGE + const connectionType = {}; + let isPoolBusy = true; + let abortController: AbortController | undefined; + const fallBackHandler = vi.fn(async (ac: AbortController) => { + abortController = ac; + return connectionType; + }); + const isIdle = vi.fn(() => !isPoolBusy); + + await cpm.getConnection({ + credentials: {}, + nodeType: 'example', + nodeVersion: '1', + fallBackHandler, + isIdle, + wasUsed: vi.fn(), + }); + + // ACT 1 + vi.advanceTimersByTime(ttl + cleanUpInterval * 2); + + // ASSERT 1 + if (abortController === undefined) { + expect.fail("abortController haven't been initialized"); + } + const controller = abortController; + expect(isIdle).toHaveBeenCalledWith(connectionType); + expect(controller.signal.aborted).toBe(false); + + // ACT 2 + isPoolBusy = false; + vi.advanceTimersByTime(ttl + cleanUpInterval * 2); + + // ASSERT 2 + expect(controller.signal.aborted).toBe(true); + }); + test('throws OperationsError if the fallBackHandler aborts during connection initialization', async () => { // ARRANGE const connectionType = {}; diff --git a/packages/nodes-base/utils/connection-pool-manager.ts b/packages/nodes-base/utils/connection-pool-manager.ts index 5809f27bbc9..6a6c5e3048a 100644 --- a/packages/nodes-base/utils/connection-pool-manager.ts +++ b/packages/nodes-base/utils/connection-pool-manager.ts @@ -25,6 +25,12 @@ type GetConnectionOption = RegistrationOptions & { */ fallBackHandler: (abortController: AbortController) => Promise; + /** + * Returns whether the pool can be safely cleaned up. If omitted, stale pools + * are assumed to be idle. + */ + isIdle?: (pool: Pool) => boolean; + wasUsed: (pool: Pool) => void; }; @@ -34,6 +40,7 @@ type Registration = { abortController: AbortController; + isIdle?: (pool: Pool) => boolean; /** We keep this timestamp to check if a pool hasn't been used in a while, and if it needs to be closed */ lastUsed: number; }; @@ -109,6 +116,7 @@ export class ConnectionPoolManager { value = { pool: await options.fallBackHandler(abortController), abortController, + isIdle: options.isIdle, } as Registration; // It's possible that `options.fallBackHandler` already called the abort @@ -143,8 +151,14 @@ export class ConnectionPoolManager { */ private cleanupStaleConnections() { const now = Date.now(); - for (const [key, { lastUsed }] of this.map.entries()) { + for (const [key, registration] of this.map.entries()) { + const { isIdle, lastUsed, pool } = registration; if (now - lastUsed > ttl) { + if (isIdle && !isIdle(pool)) { + registration.lastUsed = now; + this.logger.debug('ConnectionPoolManager: Found stale pool, but it is still in use.'); + continue; + } this.logger.debug('ConnectionPoolManager: Found stale pool. Cleaning it up.'); void this.cleanupConnection(key); }