Skip to content

Commit e9e4f43

Browse files
addaleaxBethGriggs
authored andcommitted
http2: fix memory leak when headers are not emitted
When headers are not emitted to JS, e.g. because of an error before that could happen, we currently still have the vector of previously received headers lying around, each one holding a reference count of 1. To fix the resulting memory leak, release them in the `Http2Stream` destructor. Also, clear the vector of headers once they have been emitted – there’s not need to keep it around, wasting memory. Backport-PR-URL: #22850 PR-URL: #21373 Reviewed-By: Tiancheng "Timothy" Gu <[email protected]>
1 parent 0f3e650 commit e9e4f43

File tree

2 files changed

+10
-11
lines changed

2 files changed

+10
-11
lines changed

src/node_http2.cc

+8-5
Original file line numberDiff line numberDiff line change
@@ -1137,8 +1137,7 @@ inline void Http2Session::HandleHeadersFrame(const nghttp2_frame* frame) {
11371137
if (stream->IsDestroyed())
11381138
return;
11391139

1140-
nghttp2_header* headers = stream->headers();
1141-
size_t count = stream->headers_count();
1140+
std::vector<nghttp2_header> headers(stream->move_headers());
11421141

11431142
Local<String> name_str;
11441143
Local<String> value_str;
@@ -1155,9 +1154,9 @@ inline void Http2Session::HandleHeadersFrame(const nghttp2_frame* frame) {
11551154
// this way for performance reasons (it's faster to generate and pass an
11561155
// array than it is to generate and pass the object).
11571156
size_t n = 0;
1158-
while (count > 0) {
1157+
while (n < headers.size()) {
11591158
size_t j = 0;
1160-
while (count > 0 && j < arraysize(argv) / 2) {
1159+
while (n < headers.size() && j < arraysize(argv) / 2) {
11611160
nghttp2_header item = headers[n++];
11621161
// The header name and value are passed as external one-byte strings
11631162
name_str =
@@ -1166,7 +1165,6 @@ inline void Http2Session::HandleHeadersFrame(const nghttp2_frame* frame) {
11661165
ExternalHeader::New<false>(env(), item.value).ToLocalChecked();
11671166
argv[j * 2] = name_str;
11681167
argv[j * 2 + 1] = value_str;
1169-
count--;
11701168
j++;
11711169
}
11721170
// For performance, we pass name and value pairs to array.protototype.push
@@ -1782,6 +1780,11 @@ Http2Stream::Http2Stream(
17821780

17831781

17841782
Http2Stream::~Http2Stream() {
1783+
for (nghttp2_header& header : current_headers_) {
1784+
nghttp2_rcbuf_decref(header.name);
1785+
nghttp2_rcbuf_decref(header.value);
1786+
}
1787+
17851788
if (session_ == nullptr)
17861789
return;
17871790
DEBUG_HTTP2STREAM(this, "tearing down stream");

src/node_http2.h

+2-6
Original file line numberDiff line numberDiff line change
@@ -653,18 +653,14 @@ class Http2Stream : public AsyncWrap,
653653
nghttp2_rcbuf* value,
654654
uint8_t flags);
655655

656-
inline nghttp2_header* headers() {
657-
return current_headers_.data();
656+
inline std::vector<nghttp2_header> move_headers() {
657+
return std::move(current_headers_);
658658
}
659659

660660
inline nghttp2_headers_category headers_category() const {
661661
return current_headers_category_;
662662
}
663663

664-
inline size_t headers_count() const {
665-
return current_headers_.size();
666-
}
667-
668664
void StartHeaders(nghttp2_headers_category category);
669665

670666
// Required for StreamBase

0 commit comments

Comments
 (0)