From 9717d747afd17d0011b0773d308aa4573a980b0b Mon Sep 17 00:00:00 2001 From: DeepView Autofix <276251120+deepview-autofix@users.noreply.github.com> Date: Sun, 19 Apr 2026 21:29:23 +0300 Subject: [PATCH] fix(round-robin-pool): use live clients length after TTL eviction The round-robin dispatcher loop captured `this[kClients].length` before iterating, but `kRemoveClient` can shrink the array during iteration when multiple clients are evicted by TTL. The stale length caused the modulus to point past the new array bounds, so `this[kClients][this[kIndex]]` returned `undefined` and the next property access (`.ttl` or `[kNeedDrain]`) threw a TypeError. Read the length dynamically on each iteration and bail out once the array is empty so the loop always indexes into valid entries. Co-Authored-By: Claude Co-Authored-By: DeepView Autofix <276251120+deepview-autofix@users.noreply.github.com> Co-Authored-By: Nikita Skovoroda Signed-off-by: Nikita Skovoroda --- lib/dispatcher/round-robin-pool.js | 6 ++-- test/round-robin-pool.js | 45 ++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 3 deletions(-) diff --git a/lib/dispatcher/round-robin-pool.js b/lib/dispatcher/round-robin-pool.js index ac08f460923..98e1e3e4d84 100644 --- a/lib/dispatcher/round-robin-pool.js +++ b/lib/dispatcher/round-robin-pool.js @@ -103,8 +103,8 @@ class RoundRobinPool extends PoolBase { // Round-robin through existing clients let checked = 0 - while (checked < clientsLength) { - this[kIndex] = (this[kIndex] + 1) % clientsLength + while (checked < clientsLength && this[kClients].length > 0) { + this[kIndex] = (this[kIndex] + 1) % this[kClients].length const client = this[kClients][this[kIndex]] // Check if client is stale (TTL expired) @@ -123,7 +123,7 @@ class RoundRobinPool extends PoolBase { } // All clients are busy, create a new one if we haven't reached the limit - if (!this[kConnections] || clientsLength < this[kConnections]) { + if (!this[kConnections] || this[kClients].length < this[kConnections]) { const dispatcher = this[kFactory](this[kUrl], this[kOptions]) this[kAddClient](dispatcher) return dispatcher diff --git a/test/round-robin-pool.js b/test/round-robin-pool.js index 1510cbbf91a..89153d57b2b 100644 --- a/test/round-robin-pool.js +++ b/test/round-robin-pool.js @@ -382,3 +382,48 @@ test('verifies round-robin kGetDispatcher cycling algorithm', async (t) => { await t.completed }) + +test('does not crash when multiple clients are evicted by TTL in one dispatch', (t) => { + const p = tspl(t, { plan: 2 }) + + const { EventEmitter } = require('node:events') + const { kNeedDrain, kAddClient, kGetDispatcher } = require('../lib/dispatcher/pool-base') + + class FakeClient extends EventEmitter { + constructor () { + super() + this.closed = false + this.destroyed = false + this[kNeedDrain] = false + } + + close (cb) { + this.closed = true + if (cb) cb() + } + + destroy (err, cb) { if (cb) cb() } + dispatch () { return true } + } + + const pool = new RoundRobinPool('http://localhost', { + connections: 10, + clientTtl: 1, + factory: () => new FakeClient() + }) + + // Seed two clients with stale TTLs so the round-robin loop tries to evict + // both in a single dispatch. Before the fix, the stale `clientsLength` + // caused the loop to index past the shrinking array and crash with + // "Cannot read properties of undefined (reading 'ttl')". + const c1 = new FakeClient() + const c2 = new FakeClient() + pool[kAddClient](c1) + pool[kAddClient](c2) + c1.ttl = Date.now() - 1000 + c2.ttl = Date.now() - 1000 + + const dispatcher = pool[kGetDispatcher]() + p.ok(dispatcher, 'dispatch returned a client without crashing') + p.ok(dispatcher instanceof FakeClient) +})