Skip to content

Commit 9cc025b

Browse files
authored
Fix clientTtl cleanup race (#4807)
1 parent fc8bb75 commit 9cc025b

3 files changed

Lines changed: 57 additions & 5 deletions

File tree

lib/dispatcher/agent.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,9 @@ class Agent extends DispatcherBase {
9292
if (connected) result.count -= 1
9393
if (result.count <= 0) {
9494
this[kClients].delete(key)
95-
result.dispatcher.close()
95+
if (!result.dispatcher.destroyed) {
96+
result.dispatcher.close()
97+
}
9698
}
9799
this[kOrigins].delete(key)
98100
}

lib/dispatcher/pool-base.js

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,9 +48,12 @@ class PoolBase extends DispatcherBase {
4848
}
4949

5050
if (this[kClosedResolve] && queue.isEmpty()) {
51-
const closeAll = new Array(this[kClients].length)
51+
const closeAll = []
5252
for (let i = 0; i < this[kClients].length; i++) {
53-
closeAll[i] = this[kClients][i].close()
53+
const client = this[kClients][i]
54+
if (!client.destroyed) {
55+
closeAll.push(client.close())
56+
}
5457
}
5558
return Promise.all(closeAll)
5659
.then(this[kClosedResolve])
@@ -119,9 +122,12 @@ class PoolBase extends DispatcherBase {
119122

120123
[kClose] () {
121124
if (this[kQueue].isEmpty()) {
122-
const closeAll = new Array(this[kClients].length)
125+
const closeAll = []
123126
for (let i = 0; i < this[kClients].length; i++) {
124-
closeAll[i] = this[kClients][i].close()
127+
const client = this[kClients][i]
128+
if (!client.destroyed) {
129+
closeAll.push(client.close())
130+
}
125131
}
126132
return Promise.all(closeAll)
127133
} else {

test/issue-4806.js

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
'use strict'
2+
3+
const { test, after } = require('node:test')
4+
const { createServer } = require('node:http')
5+
const { once } = require('node:events')
6+
const { tspl } = require('@matteo.collina/tspl')
7+
8+
const { Agent, request } = require('..')
9+
10+
// https://github.com/nodejs/undici/issues/4806
11+
test('Agent clientTtl cleanup does not trigger unhandled rejections', async (t) => {
12+
t = tspl(t, { plan: 2 })
13+
14+
const server = createServer((req, res) => {
15+
res.end('ok')
16+
})
17+
18+
after(() => server.close())
19+
20+
server.listen(0, async () => {
21+
const agent = new Agent({ clientTtl: 10 })
22+
after(async () => agent.close())
23+
24+
const onUnhandled = (err) => t.fail(err)
25+
process.once('unhandledRejection', onUnhandled)
26+
after(() => process.removeListener('unhandledRejection', onUnhandled))
27+
28+
const origin = `http://localhost:${server.address().port}`
29+
30+
const res1 = await request(origin, { dispatcher: agent })
31+
t.strictEqual(res1.statusCode, 200)
32+
33+
await new Promise(resolve => setTimeout(resolve, 20))
34+
35+
const res2 = await request(origin, { dispatcher: agent })
36+
t.strictEqual(res2.statusCode, 200)
37+
res2.body.resume()
38+
await once(res2.body, 'end')
39+
40+
await new Promise(resolve => setTimeout(resolve, 20))
41+
})
42+
43+
await t.completed
44+
})

0 commit comments

Comments
 (0)