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 20b72fc

Browse filesBrowse files
addaleaxBethGriggs
addaleax
authored and
BethGriggs
committed
http2: track memory allocated by nghttp2
Provide a custom memory allocator for nghttp2, and track memory allocated by the library with it. This makes the used-memory-per-session estimate more accurate, and allows us to track memory leaks either in nghttp2 itself or, more likely, through faulty usage on our end. It also allows us to make the per-session memory limit more accurate in the future; currently, we are not handling this in an ideal way, and instead let nghttp2 allocate what it wants, even if that goes over our limit. Backport-PR-URL: #22850 PR-URL: #21374 Refs: #21373 Refs: #21336 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
1 parent e9e4f43 commit 20b72fc
Copy full SHA for 20b72fc

File tree

Expand file treeCollapse file tree

4 files changed

+113
-13
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

4 files changed

+113
-13
lines changed
Open diff view settings
Collapse file

‎src/node_http2.cc‎

Copy file name to clipboardExpand all lines: src/node_http2.cc
+94-5Lines changed: 94 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -486,6 +486,92 @@ Http2Session::Callbacks::~Callbacks() {
486486
nghttp2_session_callbacks_del(callbacks);
487487
}
488488

489+
// Track memory allocated by nghttp2 using a custom allocator.
490+
class Http2Session::MemoryAllocatorInfo {
491+
public:
492+
explicit MemoryAllocatorInfo(Http2Session* session)
493+
: info({ session, H2Malloc, H2Free, H2Calloc, H2Realloc }) {}
494+
495+
static void* H2Malloc(size_t size, void* user_data) {
496+
return H2Realloc(nullptr, size, user_data);
497+
}
498+
499+
static void* H2Calloc(size_t nmemb, size_t size, void* user_data) {
500+
size_t real_size = MultiplyWithOverflowCheck(nmemb, size);
501+
void* mem = H2Malloc(real_size, user_data);
502+
if (mem != nullptr)
503+
memset(mem, 0, real_size);
504+
return mem;
505+
}
506+
507+
static void H2Free(void* ptr, void* user_data) {
508+
if (ptr == nullptr) return; // free(null); happens quite often.
509+
void* result = H2Realloc(ptr, 0, user_data);
510+
CHECK_EQ(result, nullptr);
511+
}
512+
513+
static void* H2Realloc(void* ptr, size_t size, void* user_data) {
514+
Http2Session* session = static_cast<Http2Session*>(user_data);
515+
size_t previous_size = 0;
516+
char* original_ptr = nullptr;
517+
518+
// We prepend each allocated buffer with a size_t containing the full
519+
// size of the allocation.
520+
if (size > 0) size += sizeof(size_t);
521+
522+
if (ptr != nullptr) {
523+
// We are free()ing or re-allocating.
524+
original_ptr = static_cast<char*>(ptr) - sizeof(size_t);
525+
previous_size = *reinterpret_cast<size_t*>(original_ptr);
526+
// This means we called StopTracking() on this pointer before.
527+
if (previous_size == 0) {
528+
// Fall back to the standard Realloc() function.
529+
char* ret = UncheckedRealloc(original_ptr, size);
530+
if (ret != nullptr)
531+
ret += sizeof(size_t);
532+
return ret;
533+
}
534+
}
535+
CHECK_GE(session->current_nghttp2_memory_, previous_size);
536+
537+
// TODO(addaleax): Add the following, and handle NGHTTP2_ERR_NOMEM properly
538+
// everywhere:
539+
//
540+
// if (size > previous_size &&
541+
// !session->IsAvailableSessionMemory(size - previous_size)) {
542+
// return nullptr;
543+
//}
544+
545+
char* mem = UncheckedRealloc(original_ptr, size);
546+
547+
if (mem != nullptr) {
548+
// Adjust the memory info counter.
549+
session->current_nghttp2_memory_ += size - previous_size;
550+
*reinterpret_cast<size_t*>(mem) = size;
551+
mem += sizeof(size_t);
552+
} else if (size == 0) {
553+
session->current_nghttp2_memory_ -= previous_size;
554+
}
555+
556+
return mem;
557+
}
558+
559+
static void StopTracking(Http2Session* session, void* ptr) {
560+
size_t* original_ptr = reinterpret_cast<size_t*>(
561+
static_cast<char*>(ptr) - sizeof(size_t));
562+
session->current_nghttp2_memory_ -= *original_ptr;
563+
*original_ptr = 0;
564+
}
565+
566+
inline nghttp2_mem* operator*() { return &info; }
567+
568+
nghttp2_mem info;
569+
};
570+
571+
void Http2Session::StopTrackingRcbuf(nghttp2_rcbuf* buf) {
572+
MemoryAllocatorInfo::StopTracking(this, buf);
573+
}
574+
489575
Http2Session::Http2Session(Environment* env,
490576
Local<Object> wrap,
491577
nghttp2_session_type type)
@@ -517,15 +603,17 @@ Http2Session::Http2Session(Environment* env,
517603
= callback_struct_saved[hasGetPaddingCallback ? 1 : 0].callbacks;
518604

519605
auto fn = type == NGHTTP2_SESSION_SERVER ?
520-
nghttp2_session_server_new2 :
521-
nghttp2_session_client_new2;
606+
nghttp2_session_server_new3 :
607+
nghttp2_session_client_new3;
608+
609+
MemoryAllocatorInfo allocator_info(this);
522610

523611
// This should fail only if the system is out of memory, which
524612
// is going to cause lots of other problems anyway, or if any
525613
// of the options are out of acceptable range, which we should
526614
// be catching before it gets this far. Either way, crash if this
527615
// fails.
528-
CHECK_EQ(fn(&session_, callbacks, this, *opts), 0);
616+
CHECK_EQ(fn(&session_, callbacks, this, *opts, *allocator_info), 0);
529617

530618
outgoing_storage_.reserve(4096);
531619
outgoing_buffers_.reserve(32);
@@ -553,6 +641,7 @@ Http2Session::~Http2Session() {
553641
Unconsume();
554642
DEBUG_HTTP2SESSION(this, "freeing nghttp2 session");
555643
nghttp2_session_del(session_);
644+
CHECK_EQ(current_nghttp2_memory_, 0);
556645
}
557646

558647
inline bool HasHttp2Observer(Environment* env) {
@@ -1160,9 +1249,9 @@ inline void Http2Session::HandleHeadersFrame(const nghttp2_frame* frame) {
11601249
nghttp2_header item = headers[n++];
11611250
// The header name and value are passed as external one-byte strings
11621251
name_str =
1163-
ExternalHeader::New<true>(env(), item.name).ToLocalChecked();
1252+
ExternalHeader::New<true>(this, item.name).ToLocalChecked();
11641253
value_str =
1165-
ExternalHeader::New<false>(env(), item.value).ToLocalChecked();
1254+
ExternalHeader::New<false>(this, item.value).ToLocalChecked();
11661255
argv[j * 2] = name_str;
11671256
argv[j * 2 + 1] = value_str;
11681257
j++;
Collapse file

‎src/node_http2.h‎

Copy file name to clipboardExpand all lines: src/node_http2.h
+13-6Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -797,6 +797,7 @@ class Http2Session : public AsyncWrap {
797797

798798
class Http2Ping;
799799
class Http2Settings;
800+
class MemoryAllocatorInfo;
800801

801802
inline void EmitStatistics();
802803

@@ -934,13 +935,15 @@ class Http2Session : public AsyncWrap {
934935
current_session_memory_ -= amount;
935936
}
936937

937-
// Returns the current session memory including the current size of both
938-
// the inflate and deflate hpack headers, the current outbound storage
939-
// queue, and pending writes.
938+
// Tell our custom memory allocator that this rcbuf is independent of
939+
// this session now, and may outlive it.
940+
void StopTrackingRcbuf(nghttp2_rcbuf* buf);
941+
942+
// Returns the current session memory including memory allocated by nghttp2,
943+
// the current outbound storage queue, and pending writes.
940944
uint64_t GetCurrentSessionMemory() {
941945
uint64_t total = current_session_memory_ + sizeof(Http2Session);
942-
total += nghttp2_session_get_hd_deflate_dynamic_table_size(session_);
943-
total += nghttp2_session_get_hd_inflate_dynamic_table_size(session_);
946+
total += current_nghttp2_memory_;
944947
total += outgoing_storage_.size();
945948
return total;
946949
}
@@ -1072,6 +1075,8 @@ class Http2Session : public AsyncWrap {
10721075
// The maximum amount of memory allocated for this session
10731076
uint64_t max_session_memory_ = DEFAULT_MAX_SESSION_MEMORY;
10741077
uint64_t current_session_memory_ = 0;
1078+
// The amount of memory allocated by nghttp2 internals
1079+
uint64_t current_nghttp2_memory_ = 0;
10751080

10761081
// The collection of active Http2Streams associated with this session
10771082
std::unordered_map<int32_t, Http2Stream*> streams_;
@@ -1274,7 +1279,8 @@ class ExternalHeader :
12741279
}
12751280

12761281
template<bool may_internalize>
1277-
static MaybeLocal<String> New(Environment* env, nghttp2_rcbuf* buf) {
1282+
static MaybeLocal<String> New(Http2Session* session, nghttp2_rcbuf* buf) {
1283+
Environment* env = session->env();
12781284
if (nghttp2_rcbuf_is_static(buf)) {
12791285
auto& static_str_map = env->isolate_data()->http2_static_strs;
12801286
v8::Eternal<v8::String>& eternal = static_str_map[buf];
@@ -1301,6 +1307,7 @@ class ExternalHeader :
13011307
return GetInternalizedString(env, vec);
13021308
}
13031309

1310+
session->StopTrackingRcbuf(buf);
13041311
ExternalHeader* h_str = new ExternalHeader(buf);
13051312
MaybeLocal<String> str = String::NewExternalOneByte(env->isolate(), h_str);
13061313
if (str.IsEmpty())
Collapse file

‎src/util-inl.h‎

Copy file name to clipboardExpand all lines: src/util-inl.h
+3-2Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -345,8 +345,9 @@ bool StringEqualNoCaseN(const char* a, const char* b, size_t length) {
345345
return true;
346346
}
347347

348-
inline size_t MultiplyWithOverflowCheck(size_t a, size_t b) {
349-
size_t ret = a * b;
348+
template <typename T>
349+
inline T MultiplyWithOverflowCheck(T a, T b) {
350+
auto ret = a * b;
350351
if (a != 0)
351352
CHECK_EQ(b, ret / a);
352353

Collapse file

‎src/util.h‎

Copy file name to clipboardExpand all lines: src/util.h
+3Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,9 @@ inline char* Calloc(size_t n);
6565
inline char* UncheckedMalloc(size_t n);
6666
inline char* UncheckedCalloc(size_t n);
6767

68+
template <typename T>
69+
inline T MultiplyWithOverflowCheck(T a, T b);
70+
6871
// Used by the allocation functions when allocation fails.
6972
// Thin wrapper around v8::Isolate::LowMemoryNotification() that checks
7073
// whether V8 is initialized.

0 commit comments

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