Skip to content

Commit 7a2e2fb

Browse files
committed
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. PR-URL: #21373 Reviewed-By: Tiancheng "Timothy" Gu <[email protected]>
1 parent d362b07 commit 7a2e2fb

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
@@ -1127,8 +1127,7 @@ void Http2Session::HandleHeadersFrame(const nghttp2_frame* frame) {
11271127
if (stream->IsDestroyed())
11281128
return;
11291129

1130-
nghttp2_header* headers = stream->headers();
1131-
size_t count = stream->headers_count();
1130+
std::vector<nghttp2_header> headers(stream->move_headers());
11321131

11331132
Local<String> name_str;
11341133
Local<String> value_str;
@@ -1145,9 +1144,9 @@ void Http2Session::HandleHeadersFrame(const nghttp2_frame* frame) {
11451144
// this way for performance reasons (it's faster to generate and pass an
11461145
// array than it is to generate and pass the object).
11471146
size_t n = 0;
1148-
while (count > 0) {
1147+
while (n < headers.size()) {
11491148
size_t j = 0;
1150-
while (count > 0 && j < arraysize(argv) / 2) {
1149+
while (n < headers.size() && j < arraysize(argv) / 2) {
11511150
nghttp2_header item = headers[n++];
11521151
// The header name and value are passed as external one-byte strings
11531152
name_str =
@@ -1156,7 +1155,6 @@ void Http2Session::HandleHeadersFrame(const nghttp2_frame* frame) {
11561155
ExternalHeader::New<false>(env(), item.value).ToLocalChecked();
11571156
argv[j * 2] = name_str;
11581157
argv[j * 2 + 1] = value_str;
1159-
count--;
11601158
j++;
11611159
}
11621160
// For performance, we pass name and value pairs to array.protototype.push
@@ -1710,6 +1708,11 @@ Http2Stream::Http2Stream(
17101708

17111709

17121710
Http2Stream::~Http2Stream() {
1711+
for (nghttp2_header& header : current_headers_) {
1712+
nghttp2_rcbuf_decref(header.name);
1713+
nghttp2_rcbuf_decref(header.value);
1714+
}
1715+
17131716
if (session_ == nullptr)
17141717
return;
17151718
Debug(this, "tearing down stream");

src/node_http2.h

+2-6
Original file line numberDiff line numberDiff line change
@@ -592,18 +592,14 @@ class Http2Stream : public AsyncWrap,
592592

593593
bool AddHeader(nghttp2_rcbuf* name, nghttp2_rcbuf* value, uint8_t flags);
594594

595-
inline nghttp2_header* headers() {
596-
return current_headers_.data();
595+
inline std::vector<nghttp2_header> move_headers() {
596+
return std::move(current_headers_);
597597
}
598598

599599
inline nghttp2_headers_category headers_category() const {
600600
return current_headers_category_;
601601
}
602602

603-
inline size_t headers_count() const {
604-
return current_headers_.size();
605-
}
606-
607603
void StartHeaders(nghttp2_headers_category category);
608604

609605
// Required for StreamBase

0 commit comments

Comments
 (0)