Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 2a18859

Browse filesBrowse files
RafaelGSSruyadorno
authored andcommitted
http2: fix memory leak on nghttp2 hd threshold
PR-URL: #41502 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
1 parent 7f7bcd7 commit 2a18859
Copy full SHA for 2a18859

File tree

Expand file treeCollapse file tree

3 files changed

+72
-0
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

3 files changed

+72
-0
lines changed
Open diff view settings
Collapse file

‎src/node_http2.cc‎

Copy file name to clipboardExpand all lines: src/node_http2.cc
+20Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1009,6 +1009,21 @@ int Http2Session::OnInvalidFrame(nghttp2_session* handle,
10091009
return 0;
10101010
}
10111011

1012+
// Remove the headers reference.
1013+
// Implicitly calls nghttp2_rcbuf_decref
1014+
void Http2Session::DecrefHeaders(const nghttp2_frame* frame) {
1015+
int32_t id = GetFrameID(frame);
1016+
BaseObjectPtr<Http2Stream> stream = FindStream(id);
1017+
1018+
if (stream && !stream->is_destroyed() && stream->headers_count() > 0) {
1019+
Debug(this, "freeing headers for stream %d", id);
1020+
stream->ClearHeaders();
1021+
CHECK_EQ(stream->headers_count(), 0);
1022+
DecrementCurrentSessionMemory(stream->current_headers_length_);
1023+
stream->current_headers_length_ = 0;
1024+
}
1025+
}
1026+
10121027
// If nghttp2 is unable to send a queued up frame, it will call this callback
10131028
// to let us know. If the failure occurred because we are in the process of
10141029
// closing down the session or stream, we go ahead and ignore it. We don't
@@ -1029,6 +1044,11 @@ int Http2Session::OnFrameNotSent(nghttp2_session* handle,
10291044
error_code == NGHTTP2_ERR_STREAM_CLOSED ||
10301045
error_code == NGHTTP2_ERR_STREAM_CLOSING ||
10311046
session->js_fields_->frame_error_listener_count == 0) {
1047+
// Nghttp2 contains header limit of 65536. When this value is exceeded the
1048+
// pipeline is stopped and we should remove the current headers reference
1049+
// to destroy the session completely.
1050+
// Further information see: https://github.com/nodejs/node/issues/35233
1051+
session->DecrefHeaders(frame);
10321052
return 0;
10331053
}
10341054

Collapse file

‎src/node_http2.h‎

Copy file name to clipboardExpand all lines: src/node_http2.h
+6Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -401,6 +401,10 @@ class Http2Stream : public AsyncWrap,
401401
size_t i = 0;
402402
for (const auto& header : current_headers_ )
403403
fn(header, i++);
404+
ClearHeaders();
405+
}
406+
407+
void ClearHeaders() {
404408
current_headers_.clear();
405409
}
406410

@@ -787,6 +791,8 @@ class Http2Session : public AsyncWrap,
787791
void HandleAltSvcFrame(const nghttp2_frame* frame);
788792
void HandleOriginFrame(const nghttp2_frame* frame);
789793

794+
void DecrefHeaders(const nghttp2_frame* frame);
795+
790796
// nghttp2 callbacks
791797
static int OnBeginHeadersCallback(
792798
nghttp2_session* session,
Collapse file
+46Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
if (!common.hasCrypto)
5+
common.skip('missing crypto');
6+
const h2 = require('http2');
7+
8+
const server = h2.createServer();
9+
10+
server.on('stream', common.mustNotCall());
11+
server.on('error', common.mustNotCall());
12+
13+
server.listen(0, common.mustCall(() => {
14+
15+
// Setting the maxSendHeaderBlockLength > nghttp2 threshold
16+
// cause a 'sessionError' and no memory leak when session destroy
17+
const options = {
18+
maxSendHeaderBlockLength: 100000
19+
};
20+
21+
const client = h2.connect(`http://localhost:${server.address().port}`,
22+
options);
23+
client.on('error', common.expectsError({
24+
code: 'ERR_HTTP2_SESSION_ERROR',
25+
name: 'Error',
26+
message: 'Session closed with error code 9'
27+
}));
28+
29+
const req = client.request({
30+
// Greater than 65536 bytes
31+
'test-header': 'A'.repeat(90000)
32+
});
33+
req.on('response', common.mustNotCall());
34+
35+
req.on('close', common.mustCall(() => {
36+
client.close();
37+
server.close();
38+
}));
39+
40+
req.on('error', common.expectsError({
41+
code: 'ERR_HTTP2_SESSION_ERROR',
42+
name: 'Error',
43+
message: 'Session closed with error code 9'
44+
}));
45+
req.end();
46+
}));

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.