Skip to content

Commit 5a71e79

Browse files
addaleaxtargos
authored andcommitted
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. PR-URL: #21374 Refs: #21373 Refs: #21336 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: Tiancheng "Timothy" Gu <[email protected]>
1 parent de195d5 commit 5a71e79

File tree

4 files changed

+113
-13
lines changed

4 files changed

+113
-13
lines changed

src/node_http2.cc

+94-5
Original file line numberDiff line numberDiff line change
@@ -451,6 +451,92 @@ Http2Session::Callbacks::~Callbacks() {
451451
nghttp2_session_callbacks_del(callbacks);
452452
}
453453

454+
// Track memory allocated by nghttp2 using a custom allocator.
455+
class Http2Session::MemoryAllocatorInfo {
456+
public:
457+
explicit MemoryAllocatorInfo(Http2Session* session)
458+
: info({ session, H2Malloc, H2Free, H2Calloc, H2Realloc }) {}
459+
460+
static void* H2Malloc(size_t size, void* user_data) {
461+
return H2Realloc(nullptr, size, user_data);
462+
}
463+
464+
static void* H2Calloc(size_t nmemb, size_t size, void* user_data) {
465+
size_t real_size = MultiplyWithOverflowCheck(nmemb, size);
466+
void* mem = H2Malloc(real_size, user_data);
467+
if (mem != nullptr)
468+
memset(mem, 0, real_size);
469+
return mem;
470+
}
471+
472+
static void H2Free(void* ptr, void* user_data) {
473+
if (ptr == nullptr) return; // free(null); happens quite often.
474+
void* result = H2Realloc(ptr, 0, user_data);
475+
CHECK_EQ(result, nullptr);
476+
}
477+
478+
static void* H2Realloc(void* ptr, size_t size, void* user_data) {
479+
Http2Session* session = static_cast<Http2Session*>(user_data);
480+
size_t previous_size = 0;
481+
char* original_ptr = nullptr;
482+
483+
// We prepend each allocated buffer with a size_t containing the full
484+
// size of the allocation.
485+
if (size > 0) size += sizeof(size_t);
486+
487+
if (ptr != nullptr) {
488+
// We are free()ing or re-allocating.
489+
original_ptr = static_cast<char*>(ptr) - sizeof(size_t);
490+
previous_size = *reinterpret_cast<size_t*>(original_ptr);
491+
// This means we called StopTracking() on this pointer before.
492+
if (previous_size == 0) {
493+
// Fall back to the standard Realloc() function.
494+
char* ret = UncheckedRealloc(original_ptr, size);
495+
if (ret != nullptr)
496+
ret += sizeof(size_t);
497+
return ret;
498+
}
499+
}
500+
CHECK_GE(session->current_nghttp2_memory_, previous_size);
501+
502+
// TODO(addaleax): Add the following, and handle NGHTTP2_ERR_NOMEM properly
503+
// everywhere:
504+
//
505+
// if (size > previous_size &&
506+
// !session->IsAvailableSessionMemory(size - previous_size)) {
507+
// return nullptr;
508+
//}
509+
510+
char* mem = UncheckedRealloc(original_ptr, size);
511+
512+
if (mem != nullptr) {
513+
// Adjust the memory info counter.
514+
session->current_nghttp2_memory_ += size - previous_size;
515+
*reinterpret_cast<size_t*>(mem) = size;
516+
mem += sizeof(size_t);
517+
} else if (size == 0) {
518+
session->current_nghttp2_memory_ -= previous_size;
519+
}
520+
521+
return mem;
522+
}
523+
524+
static void StopTracking(Http2Session* session, void* ptr) {
525+
size_t* original_ptr = reinterpret_cast<size_t*>(
526+
static_cast<char*>(ptr) - sizeof(size_t));
527+
session->current_nghttp2_memory_ -= *original_ptr;
528+
*original_ptr = 0;
529+
}
530+
531+
inline nghttp2_mem* operator*() { return &info; }
532+
533+
nghttp2_mem info;
534+
};
535+
536+
void Http2Session::StopTrackingRcbuf(nghttp2_rcbuf* buf) {
537+
MemoryAllocatorInfo::StopTracking(this, buf);
538+
}
539+
454540
Http2Session::Http2Session(Environment* env,
455541
Local<Object> wrap,
456542
nghttp2_session_type type)
@@ -482,15 +568,17 @@ Http2Session::Http2Session(Environment* env,
482568
= callback_struct_saved[hasGetPaddingCallback ? 1 : 0].callbacks;
483569

484570
auto fn = type == NGHTTP2_SESSION_SERVER ?
485-
nghttp2_session_server_new2 :
486-
nghttp2_session_client_new2;
571+
nghttp2_session_server_new3 :
572+
nghttp2_session_client_new3;
573+
574+
MemoryAllocatorInfo allocator_info(this);
487575

488576
// This should fail only if the system is out of memory, which
489577
// is going to cause lots of other problems anyway, or if any
490578
// of the options are out of acceptable range, which we should
491579
// be catching before it gets this far. Either way, crash if this
492580
// fails.
493-
CHECK_EQ(fn(&session_, callbacks, this, *opts), 0);
581+
CHECK_EQ(fn(&session_, callbacks, this, *opts, *allocator_info), 0);
494582

495583
outgoing_storage_.reserve(4096);
496584
outgoing_buffers_.reserve(32);
@@ -502,6 +590,7 @@ Http2Session::~Http2Session() {
502590
for (const auto& stream : streams_)
503591
stream.second->session_ = nullptr;
504592
nghttp2_session_del(session_);
593+
CHECK_EQ(current_nghttp2_memory_, 0);
505594
}
506595

507596
std::string Http2Session::diagnostic_name() const {
@@ -1150,9 +1239,9 @@ void Http2Session::HandleHeadersFrame(const nghttp2_frame* frame) {
11501239
nghttp2_header item = headers[n++];
11511240
// The header name and value are passed as external one-byte strings
11521241
name_str =
1153-
ExternalHeader::New<true>(env(), item.name).ToLocalChecked();
1242+
ExternalHeader::New<true>(this, item.name).ToLocalChecked();
11541243
value_str =
1155-
ExternalHeader::New<false>(env(), item.value).ToLocalChecked();
1244+
ExternalHeader::New<false>(this, item.value).ToLocalChecked();
11561245
argv[j * 2] = name_str;
11571246
argv[j * 2 + 1] = value_str;
11581247
j++;

src/node_http2.h

+13-6
Original file line numberDiff line numberDiff line change
@@ -721,6 +721,7 @@ class Http2Session : public AsyncWrap, public StreamListener {
721721

722722
class Http2Ping;
723723
class Http2Settings;
724+
class MemoryAllocatorInfo;
724725

725726
void EmitStatistics();
726727

@@ -849,13 +850,15 @@ class Http2Session : public AsyncWrap, public StreamListener {
849850
current_session_memory_ -= amount;
850851
}
851852

852-
// Returns the current session memory including the current size of both
853-
// the inflate and deflate hpack headers, the current outbound storage
854-
// queue, and pending writes.
853+
// Tell our custom memory allocator that this rcbuf is independent of
854+
// this session now, and may outlive it.
855+
void StopTrackingRcbuf(nghttp2_rcbuf* buf);
856+
857+
// Returns the current session memory including memory allocated by nghttp2,
858+
// the current outbound storage queue, and pending writes.
855859
uint64_t GetCurrentSessionMemory() {
856860
uint64_t total = current_session_memory_ + sizeof(Http2Session);
857-
total += nghttp2_session_get_hd_deflate_dynamic_table_size(session_);
858-
total += nghttp2_session_get_hd_inflate_dynamic_table_size(session_);
861+
total += current_nghttp2_memory_;
859862
total += outgoing_storage_.size();
860863
return total;
861864
}
@@ -987,6 +990,8 @@ class Http2Session : public AsyncWrap, public StreamListener {
987990
// The maximum amount of memory allocated for this session
988991
uint64_t max_session_memory_ = DEFAULT_MAX_SESSION_MEMORY;
989992
uint64_t current_session_memory_ = 0;
993+
// The amount of memory allocated by nghttp2 internals
994+
uint64_t current_nghttp2_memory_ = 0;
990995

991996
// The collection of active Http2Streams associated with this session
992997
std::unordered_map<int32_t, Http2Stream*> streams_;
@@ -1178,7 +1183,8 @@ class ExternalHeader :
11781183
}
11791184

11801185
template <bool may_internalize>
1181-
static MaybeLocal<String> New(Environment* env, nghttp2_rcbuf* buf) {
1186+
static MaybeLocal<String> New(Http2Session* session, nghttp2_rcbuf* buf) {
1187+
Environment* env = session->env();
11821188
if (nghttp2_rcbuf_is_static(buf)) {
11831189
auto& static_str_map = env->isolate_data()->http2_static_strs;
11841190
v8::Eternal<v8::String>& eternal = static_str_map[buf];
@@ -1205,6 +1211,7 @@ class ExternalHeader :
12051211
return GetInternalizedString(env, vec);
12061212
}
12071213

1214+
session->StopTrackingRcbuf(buf);
12081215
ExternalHeader* h_str = new ExternalHeader(buf);
12091216
MaybeLocal<String> str = String::NewExternalOneByte(env->isolate(), h_str);
12101217
if (str.IsEmpty())

src/util-inl.h

+3-2
Original file line numberDiff line numberDiff line change
@@ -320,8 +320,9 @@ bool StringEqualNoCaseN(const char* a, const char* b, size_t length) {
320320
return true;
321321
}
322322

323-
inline size_t MultiplyWithOverflowCheck(size_t a, size_t b) {
324-
size_t ret = a * b;
323+
template <typename T>
324+
inline T MultiplyWithOverflowCheck(T a, T b) {
325+
auto ret = a * b;
325326
if (a != 0)
326327
CHECK_EQ(b, ret / a);
327328

src/util.h

+3
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,9 @@ inline char* Calloc(size_t n);
6767
inline char* UncheckedMalloc(size_t n);
6868
inline char* UncheckedCalloc(size_t n);
6969

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

0 commit comments

Comments
 (0)