Skip to content
This repository was archived by the owner on Mar 4, 2020. It is now read-only.

Commit 1ca642c

Browse files
addaleaxdeepak1556
authored andcommitted
http2: fix tracking received data for maxSessionMemory
Track received data correctly. Specifically, for the buffer that is used for receiving data, we previously would try to increment the current memory usage by its length, and later decrement it by that, but in the meantime the buffer had been turned over to V8 and its length reset to zero. This gave the impression that more and more memory was consumed by the HTTP/2 session when it was in fact not. Fixes: nodejs/node#27416 Refs: nodejs/node#26207 PR-URL: nodejs/node#27914 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Rich Trott <[email protected]>
1 parent 9027c0f commit 1ca642c

File tree

2 files changed

+48
-2
lines changed

2 files changed

+48
-2
lines changed

src/node_http2.cc

+2-2
Original file line numberDiff line numberDiff line change
@@ -1702,7 +1702,7 @@ void Http2Session::OnStreamRead(ssize_t nread, const uv_buf_t& buf_) {
17021702
// Shrink to the actual amount of used data.
17031703
buf.Resize(nread);
17041704

1705-
IncrementCurrentSessionMemory(buf.size());
1705+
IncrementCurrentSessionMemory(nread);
17061706

17071707
// Makre sure that there was no read previously active.
17081708
CHECK_NULL(stream_buf_.base);
@@ -1735,7 +1735,7 @@ void Http2Session::OnStreamRead(ssize_t nread, const uv_buf_t& buf_) {
17351735

17361736
// Since we are finished handling this write, reset the stream buffer.
17371737
// The memory has either been free()d or was handed over to V8.
1738-
DecrementCurrentSessionMemory(buf.size());
1738+
DecrementCurrentSessionMemory(nread);
17391739

17401740
stream_buf_ab_ = Local<ArrayBuffer>();
17411741
stream_buf_ = uv_buf_init(nullptr, 0);
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
'use strict';
2+
const common = require('../common');
3+
if (!common.hasCrypto)
4+
common.skip('missing crypto');
5+
const http2 = require('http2');
6+
7+
// Regression test for https://github.com/nodejs/node/issues/27416.
8+
// Check that received data is accounted for correctly in the maxSessionMemory
9+
// mechanism.
10+
11+
const bodyLength = 8192;
12+
const maxSessionMemory = 1; // 1 MB
13+
const requestCount = 1000;
14+
15+
const server = http2.createServer({ maxSessionMemory });
16+
server.on('stream', (stream) => {
17+
stream.respond();
18+
stream.end();
19+
});
20+
21+
server.listen(common.mustCall(() => {
22+
const client = http2.connect(`http://localhost:${server.address().port}`, {
23+
maxSessionMemory
24+
});
25+
26+
function request() {
27+
return new Promise((resolve, reject) => {
28+
const stream = client.request({
29+
':method': 'POST',
30+
'content-length': bodyLength
31+
});
32+
stream.on('error', reject);
33+
stream.on('response', resolve);
34+
stream.end('a'.repeat(bodyLength));
35+
});
36+
}
37+
38+
(async () => {
39+
for (let i = 0; i < requestCount; i++) {
40+
await request();
41+
}
42+
43+
client.close();
44+
server.close();
45+
})().then(common.mustCall());
46+
}));

0 commit comments

Comments
 (0)