Skip to content

Commit 59940c8

Browse files
styflestyfle
authored
fix: agent memory leak (#4223)
* fix: agent memory leak * fix: mock-agent
1 parent 1262f61 commit 59940c8

2 files changed

Lines changed: 33 additions & 23 deletions

File tree

‎lib/dispatcher/agent.js‎

Lines changed: 25 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -45,22 +45,35 @@ class Agent extends DispatcherBase {
4545
}
4646

4747
this[kOnConnect] = (origin, targets) => {
48+
const result = this[kClients].get(origin)
49+
if (result) {
50+
result.count += 1
51+
}
4852
this.emit('connect', origin, [this, ...targets])
4953
}
5054

5155
this[kOnDisconnect] = (origin, targets, err) => {
56+
const result = this[kClients].get(origin)
57+
if (result) {
58+
result.count -= 1
59+
if (result.count <= 0) {
60+
this[kClients].delete(origin)
61+
result.dispatcher.destroy()
62+
}
63+
}
5264
this.emit('disconnect', origin, [this, ...targets], err)
5365
}
5466

5567
this[kOnConnectionError] = (origin, targets, err) => {
68+
// TODO: should this decrement result.count here?
5669
this.emit('connectionError', origin, [this, ...targets], err)
5770
}
5871
}
5972

6073
get [kRunning] () {
6174
let ret = 0
62-
for (const client of this[kClients].values()) {
63-
ret += client[kRunning]
75+
for (const { dispatcher } of this[kClients].values()) {
76+
ret += dispatcher[kRunning]
6477
}
6578
return ret
6679
}
@@ -73,28 +86,25 @@ class Agent extends DispatcherBase {
7386
throw new InvalidArgumentError('opts.origin must be a non-empty string or URL.')
7487
}
7588

76-
let dispatcher = this[kClients].get(key)
77-
89+
const result = this[kClients].get(key)
90+
let dispatcher = result && result.dispatcher
7891
if (!dispatcher) {
7992
dispatcher = this[kFactory](opts.origin, this[kOptions])
8093
.on('drain', this[kOnDrain])
8194
.on('connect', this[kOnConnect])
8295
.on('disconnect', this[kOnDisconnect])
8396
.on('connectionError', this[kOnConnectionError])
8497

85-
// This introduces a tiny memory leak, as dispatchers are never removed from the map.
86-
// TODO(mcollina): remove te timer when the client/pool do not have any more
87-
// active connections.
88-
this[kClients].set(key, dispatcher)
98+
this[kClients].set(key, { count: 0, dispatcher })
8999
}
90100

91101
return dispatcher.dispatch(opts, handler)
92102
}
93103

94104
async [kClose] () {
95105
const closePromises = []
96-
for (const client of this[kClients].values()) {
97-
closePromises.push(client.close())
106+
for (const { dispatcher } of this[kClients].values()) {
107+
closePromises.push(dispatcher.close())
98108
}
99109
this[kClients].clear()
100110

@@ -103,8 +113,8 @@ class Agent extends DispatcherBase {
103113

104114
async [kDestroy] (err) {
105115
const destroyPromises = []
106-
for (const client of this[kClients].values()) {
107-
destroyPromises.push(client.destroy(err))
116+
for (const { dispatcher } of this[kClients].values()) {
117+
destroyPromises.push(dispatcher.destroy(err))
108118
}
109119
this[kClients].clear()
110120

@@ -113,9 +123,9 @@ class Agent extends DispatcherBase {
113123

114124
get stats () {
115125
const allClientStats = {}
116-
for (const client of this[kClients].values()) {
117-
if (client.stats) {
118-
allClientStats[client[kUrl].origin] = client.stats
126+
for (const { dispatcher } of this[kClients].values()) {
127+
if (dispatcher.stats) {
128+
allClientStats[dispatcher[kUrl].origin] = dispatcher.stats
119129
}
120130
}
121131
return allClientStats

‎lib/mock/mock-agent.js‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ class MockAgent extends Dispatcher {
159159
}
160160

161161
[kMockAgentSet] (origin, dispatcher) {
162-
this[kClients].set(origin, dispatcher)
162+
this[kClients].set(origin, { count: 0, dispatcher })
163163
}
164164

165165
[kFactory] (origin) {
@@ -171,9 +171,9 @@ class MockAgent extends Dispatcher {
171171

172172
[kMockAgentGet] (origin) {
173173
// First check if we can immediately find it
174-
const client = this[kClients].get(origin)
175-
if (client) {
176-
return client
174+
const result = this[kClients].get(origin)
175+
if (result?.dispatcher) {
176+
return result.dispatcher
177177
}
178178

179179
// If the origin is not a string create a dummy parent pool and return to user
@@ -184,11 +184,11 @@ class MockAgent extends Dispatcher {
184184
}
185185

186186
// If we match, create a pool and assign the same dispatches
187-
for (const [keyMatcher, nonExplicitDispatcher] of Array.from(this[kClients])) {
188-
if (nonExplicitDispatcher && typeof keyMatcher !== 'string' && matchValue(keyMatcher, origin)) {
187+
for (const [keyMatcher, result] of Array.from(this[kClients])) {
188+
if (result && typeof keyMatcher !== 'string' && matchValue(keyMatcher, origin)) {
189189
const dispatcher = this[kFactory](origin)
190190
this[kMockAgentSet](origin, dispatcher)
191-
dispatcher[kDispatches] = nonExplicitDispatcher[kDispatches]
191+
dispatcher[kDispatches] = result.dispatcher[kDispatches]
192192
return dispatcher
193193
}
194194
}
@@ -202,7 +202,7 @@ class MockAgent extends Dispatcher {
202202
const mockAgentClients = this[kClients]
203203

204204
return Array.from(mockAgentClients.entries())
205-
.flatMap(([origin, scope]) => scope[kDispatches].map(dispatch => ({ ...dispatch, origin })))
205+
.flatMap(([origin, result]) => result.dispatcher[kDispatches].map(dispatch => ({ ...dispatch, origin })))
206206
.filter(({ pending }) => pending)
207207
}
208208

0 commit comments

Comments
 (0)