Skip to content

Commit dee882e

Browse files
santigimenodanielleadams
authored andcommitted
src: let http2 streams end after session close
After the stream has been marked as closed by the nghttp2 stack, there might be still pending data to be sent: trailing headers is an example of this. In that case, avoid reentering the nghttp2 stack synchronously to allow the data to be written before actually closing the stream. Fixes: #42713 PR-URL: #45153 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Paolo Insogna <paolo@cowtech.it> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
1 parent a59c907 commit dee882e

File tree

2 files changed

+60
-0
lines changed

2 files changed

+60
-0
lines changed

src/node_http2.cc

+11
Original file line numberDiff line numberDiff line change
@@ -1120,6 +1120,17 @@ int Http2Session::OnStreamClose(nghttp2_session* handle,
11201120
if (!stream || stream->is_destroyed())
11211121
return 0;
11221122

1123+
// Don't close synchronously in case there's pending data to be written. This
1124+
// may happen when writing trailing headers.
1125+
if (code == NGHTTP2_NO_ERROR && nghttp2_session_want_write(handle) &&
1126+
!env->is_stopping()) {
1127+
env->SetImmediate([handle, id, code, user_data](Environment* env) {
1128+
OnStreamClose(handle, id, code, user_data);
1129+
});
1130+
1131+
return 0;
1132+
}
1133+
11231134
stream->Close(code);
11241135

11251136
// It is possible for the stream close to occur before the stream is
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
if (!common.hasCrypto)
5+
common.skip('missing crypto');
6+
const assert = require('assert');
7+
const http2 = require('http2');
8+
9+
const {
10+
HTTP2_HEADER_PATH,
11+
HTTP2_HEADER_STATUS,
12+
HTTP2_HEADER_METHOD,
13+
} = http2.constants;
14+
15+
const server = http2.createServer();
16+
server.on('stream', common.mustCall((stream) => {
17+
server.close();
18+
stream.session.close();
19+
stream.on('wantTrailers', common.mustCall(() => {
20+
stream.sendTrailers({ xyz: 'abc' });
21+
}));
22+
23+
stream.respond({ [HTTP2_HEADER_STATUS]: 200 }, { waitForTrailers: true });
24+
stream.write('some data');
25+
stream.end();
26+
}));
27+
28+
server.listen(0, common.mustCall(() => {
29+
const port = server.address().port;
30+
const client = http2.connect(`http://localhost:${port}`);
31+
client.socket.on('close', common.mustCall());
32+
const req = client.request({
33+
[HTTP2_HEADER_PATH]: '/',
34+
[HTTP2_HEADER_METHOD]: 'POST'
35+
});
36+
req.end();
37+
req.on('response', common.mustCall());
38+
let data = '';
39+
req.on('data', (chunk) => {
40+
data += chunk;
41+
});
42+
req.on('end', common.mustCall(() => {
43+
assert.strictEqual(data, 'some data');
44+
}));
45+
req.on('trailers', common.mustCall((headers) => {
46+
assert.strictEqual(headers.xyz, 'abc');
47+
}));
48+
req.on('close', common.mustCall());
49+
}));

0 commit comments

Comments
 (0)