Remove timers in agent.js (#2536)

Signed-off-by: Matteo Collina <hello@matteocollina.com>
nodejs · Dec 22, 2023 · 36e8131 · 36e8131
1 parent 5f4d435
commit 36e8131
Showing 1 changed file with 5 additions and 25 deletions.
diff --git 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)