Skip to content

Commit

Permalink
fix(#2364): concurrent aborts (#3005)
Browse files Browse the repository at this point in the history
* fix: concurrent aborts

* refactor: wording
  • Loading branch information
metcoder95 authored Mar 27, 2024
1 parent 7485cd9 commit fa5bcb4
Show file tree
Hide file tree
Showing 2 changed files with 97 additions and 1 deletion.
13 changes: 12 additions & 1 deletion lib/dispatcher/client-h2.js
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,18 @@ function writeH2 (client, request) {
const { [HTTP2_HEADER_STATUS]: statusCode, ...realHeaders } = headers
request.onResponseStarted()

// Due to the stream nature, it is possible we face a race condition
// where the stream has been assigned, but the request has been aborted
// the request remains in-flight and headers hasn't been received yet
// for those scenarios, best effort is to destroy the stream immediately
// as there's no value to keep it open.
if (request.aborted || request.completed) {
const err = new RequestAbortedError()
errorRequest(client, request, err)
util.destroy(stream, err)
return
}

if (request.onHeaders(Number(statusCode), realHeaders, stream.resume.bind(stream), '') === false) {
stream.pause()
}
Expand Down Expand Up @@ -426,7 +438,6 @@ function writeH2 (client, request) {

stream.once('close', () => {
session[kOpenStreams] -= 1
// TODO(HTTP/2): unref only if current streams count is 0
if (session[kOpenStreams] === 0) {
session.unref()
}
Expand Down
85 changes: 85 additions & 0 deletions test/http2.js
Original file line number Diff line number Diff line change
Expand Up @@ -1294,3 +1294,88 @@ test('Should throw informational error on half-closed streams (remote)', async t
t.strictEqual(err.code, 'UND_ERR_INFO')
})
})

test('#2364 - Concurrent aborts', async t => {
const server = createSecureServer(pem)

server.on('stream', (stream, headers, _flags, rawHeaders) => {
t.strictEqual(headers['x-my-header'], 'foo')
t.strictEqual(headers[':method'], 'GET')
setTimeout(() => {
stream.respond({
'content-type': 'text/plain; charset=utf-8',
'x-custom-h2': 'hello',
':status': 200
})
stream.end('hello h2!')
}, 100)
})

server.listen(0)
await once(server, 'listening')

const client = new Client(`https://localhost:${server.address().port}`, {
connect: {
rejectUnauthorized: false
},
allowH2: true
})

t = tspl(t, { plan: 18 })
after(() => server.close())
after(() => client.close())
const controller = new AbortController()

client.request({
path: '/',
method: 'GET',
headers: {
'x-my-header': 'foo'
}
}, (err, response) => {
t.ifError(err)
t.strictEqual(response.headers['content-type'], 'text/plain; charset=utf-8')
t.strictEqual(response.headers['x-custom-h2'], 'hello')
t.strictEqual(response.statusCode, 200)
response.body.dump()
})

client.request({
path: '/',
method: 'GET',
headers: {
'x-my-header': 'foo'
},
signal: controller.signal
}, (err, response) => {
t.strictEqual(err.name, 'AbortError')
})

client.request({
path: '/',
method: 'GET',
headers: {
'x-my-header': 'foo'
}
}, (err, response) => {
t.ifError(err)
t.strictEqual(response.headers['content-type'], 'text/plain; charset=utf-8')
t.strictEqual(response.headers['x-custom-h2'], 'hello')
t.strictEqual(response.statusCode, 200)
})

client.request({
path: '/',
method: 'GET',
headers: {
'x-my-header': 'foo'
},
signal: controller.signal
}, (err, response) => {
t.strictEqual(err.name, 'AbortError')
})

controller.abort()

await t.completed
})

0 comments on commit fa5bcb4

Please sign in to comment.