diff --git a/lib/agent.js b/lib/agent.js index 4ac34d26335..d5163507526 100644 --- a/lib/agent.js +++ b/lib/agent.js @@ -1,13 +1,12 @@ 'use strict' const { InvalidArgumentError } = require('./core/errors') -const { kClients, kRunning, kClose, kDestroy, kDispatch, kInterceptors } = require('./core/symbols') +const { kClients, kRunning, kClose, kDestroy, kDispatch, kInterceptors, kBusy } = require('./core/symbols') const DispatcherBase = require('./dispatcher-base') const Pool = require('./pool') const Client = require('./client') const util = require('./core/util') const createRedirectInterceptor = require('./interceptor/redirectInterceptor') -const { WeakRef, FinalizationRegistry } = require('./compat/dispatcher-weakref')() const kOnConnect = Symbol('onConnect') const kOnDisconnect = Symbol('onDisconnect') @@ -15,8 +14,8 @@ const kOnConnectionError = Symbol('onConnectionError') const kMaxRedirections = Symbol('maxRedirections') const kOnDrain = Symbol('onDrain') const kFactory = Symbol('factory') -const kFinalizer = Symbol('finalizer') const kOptions = Symbol('options') +const kDeleteScheduled = Symbol('deleteScheduled') function defaultFactory (origin, opts) { return opts && opts.connections === 1 @@ -55,12 +54,6 @@ class Agent extends DispatcherBase { this[kMaxRedirections] = maxRedirections this[kFactory] = factory this[kClients] = new Map() - this[kFinalizer] = new FinalizationRegistry(/* istanbul ignore next: gc is undeterministic */ key => { - const ref = this[kClients].get(key) - if (ref !== undefined && ref.deref() === undefined) { - this[kClients].delete(key) - } - }) const agent = this @@ -83,12 +76,8 @@ class Agent extends DispatcherBase { get [kRunning] () { let ret = 0 - for (const ref of this[kClients].values()) { - const client = ref.deref() - /* istanbul ignore next: gc is undeterministic */ - if (client) { - ret += client[kRunning] - } + for (const client of this[kClients].values()) { + ret += client[kRunning] } return ret } @@ -101,18 +90,38 @@ class Agent extends DispatcherBase { throw new InvalidArgumentError('opts.origin must be a non-empty string or URL.') } - const ref = this[kClients].get(key) + let dispatcher = this[kClients].get(key) - let dispatcher = ref ? ref.deref() : null if (!dispatcher) { dispatcher = this[kFactory](opts.origin, this[kOptions]) - .on('drain', this[kOnDrain]) + .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('connect', this[kOnConnect]) .on('disconnect', this[kOnDisconnect]) .on('connectionError', this[kOnConnectionError]) - this[kClients].set(key, new WeakRef(dispatcher)) - this[kFinalizer].register(dispatcher, key) + this[kClients].set(key, dispatcher) + } else if (dispatcher[kDeleteScheduled]) { + clearTimeout(dispatcher[kDeleteScheduled]) + dispatcher[kDeleteScheduled] = null } return dispatcher.dispatch(opts, handler) @@ -120,28 +129,20 @@ class Agent extends DispatcherBase { async [kClose] () { const closePromises = [] - for (const ref of this[kClients].values()) { - const client = ref.deref() - /* istanbul ignore else: gc is undeterministic */ - if (client) { - this[kFinalizer].unregister(client) - closePromises.push(client.close()) - } + for (const client of this[kClients].values()) { + closePromises.push(client.close()) } + this[kClients].clear() await Promise.all(closePromises) } async [kDestroy] (err) { const destroyPromises = [] - for (const ref of this[kClients].values()) { - const client = ref.deref() - /* istanbul ignore else: gc is undeterministic */ - if (client) { - this[kFinalizer].unregister(client) - destroyPromises.push(client.destroy(err)) - } + for (const client of this[kClients].values()) { + destroyPromises.push(client.destroy(err)) } + this[kClients].clear() await Promise.all(destroyPromises) } diff --git a/lib/mock/mock-agent.js b/lib/mock/mock-agent.js index 828e8af174d..4b29cd0112f 100644 --- a/lib/mock/mock-agent.js +++ b/lib/mock/mock-agent.js @@ -21,16 +21,6 @@ const Dispatcher = require('../dispatcher') const Pluralizer = require('./pluralizer') const PendingInterceptorsFormatter = require('./pending-interceptors-formatter') -class FakeWeakRef { - constructor (value) { - this.value = value - } - - deref () { - return this.value - } -} - class MockAgent extends Dispatcher { constructor (opts) { super(opts) @@ -103,7 +93,7 @@ class MockAgent extends Dispatcher { } [kMockAgentSet] (origin, dispatcher) { - this[kClients].set(origin, new FakeWeakRef(dispatcher)) + this[kClients].set(origin, dispatcher) } [kFactory] (origin) { @@ -115,9 +105,9 @@ class MockAgent extends Dispatcher { [kMockAgentGet] (origin) { // First check if we can immediately find it - const ref = this[kClients].get(origin) - if (ref) { - return ref.deref() + const client = this[kClients].get(origin) + if (client) { + return client } // If the origin is not a string create a dummy parent pool and return to user @@ -128,8 +118,7 @@ class MockAgent extends Dispatcher { } // If we match, create a pool and assign the same dispatches - for (const [keyMatcher, nonExplicitRef] of Array.from(this[kClients])) { - const nonExplicitDispatcher = nonExplicitRef.deref() + for (const [keyMatcher, nonExplicitDispatcher] of Array.from(this[kClients])) { if (nonExplicitDispatcher && typeof keyMatcher !== 'string' && matchValue(keyMatcher, origin)) { const dispatcher = this[kFactory](origin) this[kMockAgentSet](origin, dispatcher) @@ -147,7 +136,7 @@ class MockAgent extends Dispatcher { const mockAgentClients = this[kClients] return Array.from(mockAgentClients.entries()) - .flatMap(([origin, scope]) => scope.deref()[kDispatches].map(dispatch => ({ ...dispatch, origin }))) + .flatMap(([origin, scope]) => scope[kDispatches].map(dispatch => ({ ...dispatch, origin }))) .filter(({ pending }) => pending) } diff --git a/package.json b/package.json index f4f135ce8fb..9de9e1c29e4 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "undici", - "version": "6.1.0", + "version": "6.2.0", "description": "An HTTP/1.1 client, written from scratch for Node.js", "homepage": "https://undici.nodejs.org", "bugs": {