From d3867da10e07ff9540f868d97b55d9794f91dea9 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Fri, 22 Dec 2023 00:10:57 +0100 Subject: [PATCH] Remove timers in agent.js Signed-off-by: Matteo Collina --- lib/agent.js | 30 +++++------------------------- 1 file changed, 5 insertions(+), 25 deletions(-) diff --git a/lib/agent.js b/lib/agent.js index d5163507526..320d7c473d8 100644 --- a/lib/agent.js +++ b/lib/agent.js @@ -1,7 +1,7 @@ 'use strict' const { InvalidArgumentError } = require('./core/errors') -const { kClients, kRunning, kClose, kDestroy, kDispatch, kInterceptors, kBusy } = require('./core/symbols') +const { kClients, kRunning, kClose, kDestroy, kDispatch, kInterceptors } = require('./core/symbols') const DispatcherBase = require('./dispatcher-base') const Pool = require('./pool') const Client = require('./client') @@ -15,7 +15,6 @@ const kMaxRedirections = Symbol('maxRedirections') const kOnDrain = Symbol('onDrain') const kFactory = Symbol('factory') const kOptions = Symbol('options') -const kDeleteScheduled = Symbol('deleteScheduled') function defaultFactory (origin, opts) { return opts && opts.connections === 1 @@ -94,34 +93,15 @@ class Agent extends DispatcherBase { if (!dispatcher) { dispatcher = this[kFactory](opts.origin, this[kOptions]) - .on('drain', (...args) => { - this[kOnDrain](...args) - - // We remove the client if it is not busy for 5 minutes - // to avoid a long list of clients to saturate memory. - // Ideally, we could use a FinalizationRegistry here, but - // it is currently very buggy in Node.js. - // See - // * https://github.com/nodejs/node/issues/49344 - // * https://github.com/nodejs/node/issues/47748 - // TODO(mcollina): make the timeout configurable or - // use an event to remove disconnected clients. - this[kDeleteScheduled] = setTimeout(() => { - if (dispatcher[kBusy] === 0) { - this[kClients].destroy().then(() => {}) - this[kClients].delete(key) - } - }, 300_000) - this[kDeleteScheduled].unref() - }) + .on('drain', this[kOnDrain]) .on('connect', this[kOnConnect]) .on('disconnect', this[kOnDisconnect]) .on('connectionError', this[kOnConnectionError]) + // This introduces a tiny memory leak, as dispatchers are never removed from the map. + // TODO(mcollina): remove te timer when the client/pool do not have any more + // active connections. this[kClients].set(key, dispatcher) - } else if (dispatcher[kDeleteScheduled]) { - clearTimeout(dispatcher[kDeleteScheduled]) - dispatcher[kDeleteScheduled] = null } return dispatcher.dispatch(opts, handler)