Skip to content

Commit

Permalink
Remove timers in agent.js (nodejs#2536)
Browse files Browse the repository at this point in the history
Signed-off-by: Matteo Collina <[email protected]>
  • Loading branch information
mcollina authored and crysmags committed Feb 27, 2024
1 parent 5a92bb7 commit e1b9b3c
Showing 1 changed file with 5 additions and 25 deletions.
30 changes: 5 additions & 25 deletions lib/agent.js
Original file line number Diff line number Diff line change
@@ -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')
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit e1b9b3c

Please sign in to comment.