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) +})