Skip to content

Commit 3e1aa00

Browse files
committed
fix: memory leak in Agent
1 parent 2f6cc66 commit 3e1aa00

2 files changed

Lines changed: 53 additions & 14 deletions

File tree

lib/dispatcher/agent.js

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -45,22 +45,10 @@ 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-
}
5248
this.emit('connect', origin, [this, ...targets])
5349
}
5450

5551
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-
}
6452
this.emit('disconnect', origin, [this, ...targets], err)
6553
}
6654

@@ -91,8 +79,24 @@ class Agent extends DispatcherBase {
9179
if (!dispatcher) {
9280
dispatcher = this[kFactory](opts.origin, this[kOptions])
9381
.on('drain', this[kOnDrain])
94-
.on('connect', this[kOnConnect])
95-
.on('disconnect', this[kOnDisconnect])
82+
.on('connect', (origin, targets) => {
83+
const result = this[kClients].get(key)
84+
if (result) {
85+
result.count += 1
86+
}
87+
this[kOnConnect](origin, targets)
88+
})
89+
.on('disconnect', (origin, targets, err) => {
90+
const result = this[kClients].get(key)
91+
if (result) {
92+
result.count -= 1
93+
if (result.count <= 0) {
94+
this[kClients].delete(key)
95+
result.dispatcher.destroy()
96+
}
97+
}
98+
this[kOnDisconnect](origin, targets, err)
99+
})
96100
.on('connectionError', this[kOnConnectionError])
97101

98102
this[kClients].set(key, { count: 0, dispatcher })

test/issue-4244.js

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
const { test } = require('node:test')
2+
const assert = require('node:assert')
3+
const { createServer } = require('node:http')
4+
const { request, Agent, Pool } = require('..')
5+
6+
// https://github.com/nodejs/undici/issues/4424
7+
test('Agent should destroy clients of origins without active connections', async (t) => {
8+
const server = createServer({ keepAliveTimeout: 0 }, async (_req, res) => {
9+
res.setHeader('connection', 'close')
10+
res.writeHead(200)
11+
res.end('ok')
12+
}).listen(0)
13+
14+
t.after(() => server.close())
15+
16+
let p
17+
const agent = new Agent({
18+
factory: (origin, opts) => {
19+
const pool = new Pool(origin, opts)
20+
let _resolve, _reject
21+
p = new Promise((resolve, reject) => {
22+
_resolve = resolve
23+
_reject = reject
24+
})
25+
pool.on('disconnect', () => {
26+
setImmediate(() => pool.destroyed ? _resolve() : _reject(new Error('client not destroyed')))
27+
})
28+
return pool
29+
}
30+
})
31+
const { statusCode } = await request(`http://localhost:${server.address().port}`, { dispatcher: agent })
32+
assert.equal(statusCode, 200)
33+
34+
await p
35+
})

0 commit comments

Comments
 (0)