From fa8d967070c646c64f3f51812f584ee30e44c373 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Fri, 15 Jun 2018 00:52:35 +0200 Subject: [PATCH] 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. Refs: https://github.com/nodejs/node/pull/21373 Refs: https://github.com/nodejs/node/pull/21336 --- src/node_http2.cc | 99 ++++++++++++++++++++++++++++++++++++++++++++--- src/node_http2.h | 19 ++++++--- src/util-inl.h | 5 ++- src/util.h | 3 ++ 4 files changed, 113 insertions(+), 13 deletions(-) diff --git a/src/node_http2.cc b/src/node_http2.cc index 572f9e1b69de8a..c1ab584134b20a 100644 --- a/src/node_http2.cc +++ b/src/node_http2.cc @@ -451,6 +451,92 @@ Http2Session::Callbacks::~Callbacks() { nghttp2_session_callbacks_del(callbacks); } +// Track memory allocated by nghttp2 using a custom allocator. +class Http2Session::MemoryAllocatorInfo { + public: + explicit MemoryAllocatorInfo(Http2Session* session) + : info({ session, H2Malloc, H2Free, H2Calloc, H2Realloc }) {} + + static void* H2Malloc(size_t size, void* user_data) { + return H2Realloc(nullptr, size, user_data); + } + + static void* H2Calloc(size_t nmemb, size_t size, void* user_data) { + size_t real_size = MultiplyWithOverflowCheck(nmemb, size); + void* mem = H2Malloc(real_size, user_data); + if (mem != nullptr) + memset(mem, 0, real_size); + return mem; + } + + static void H2Free(void* ptr, void* user_data) { + if (ptr == nullptr) return; // free(null); happens quite often. + void* result = H2Realloc(ptr, 0, user_data); + CHECK_EQ(result, nullptr); + } + + static void* H2Realloc(void* ptr, size_t size, void* user_data) { + Http2Session* session = static_cast(user_data); + size_t previous_size = 0; + char* original_ptr = nullptr; + + // We prepend each allocated buffer with a size_t containing the full + // size of the allocation. + if (size > 0) size += sizeof(size_t); + + if (ptr != nullptr) { + // We are free()ing or re-allocating. + original_ptr = static_cast(ptr) - sizeof(size_t); + previous_size = *reinterpret_cast(original_ptr); + // This means we called StopTracking() on this pointer before. + if (previous_size == 0) { + // Fall back to the standard Realloc() function. + char* ret = UncheckedRealloc(original_ptr, size); + if (ret != nullptr) + ret += sizeof(size_t); + return ret; + } + } + CHECK_GE(session->current_nghttp2_memory_, previous_size); + + // TODO(addaleax): Add the following, and handle NGHTTP2_ERR_NOMEM properly + // everywhere: + // + // if (size > previous_size && + // !session->IsAvailableSessionMemory(size - previous_size)) { + // return nullptr; + //} + + char* mem = UncheckedRealloc(original_ptr, size); + + if (mem != nullptr) { + // Adjust the memory info counter. + session->current_nghttp2_memory_ += size - previous_size; + *reinterpret_cast(mem) = size; + mem += sizeof(size_t); + } else if (size == 0) { + session->current_nghttp2_memory_ -= previous_size; + } + + return mem; + } + + static void StopTracking(Http2Session* session, void* ptr) { + size_t* original_ptr = reinterpret_cast( + static_cast(ptr) - sizeof(size_t)); + session->current_nghttp2_memory_ -= *original_ptr; + *original_ptr = 0; + } + + inline nghttp2_mem* operator*() { return &info; } + + nghttp2_mem info; +}; + +void Http2Session::StopTrackingRcbuf(nghttp2_rcbuf* buf) { + MemoryAllocatorInfo::StopTracking(this, buf); +} + Http2Session::Http2Session(Environment* env, Local wrap, nghttp2_session_type type) @@ -482,15 +568,17 @@ Http2Session::Http2Session(Environment* env, = callback_struct_saved[hasGetPaddingCallback ? 1 : 0].callbacks; auto fn = type == NGHTTP2_SESSION_SERVER ? - nghttp2_session_server_new2 : - nghttp2_session_client_new2; + nghttp2_session_server_new3 : + nghttp2_session_client_new3; + + MemoryAllocatorInfo allocator_info(this); // This should fail only if the system is out of memory, which // is going to cause lots of other problems anyway, or if any // of the options are out of acceptable range, which we should // be catching before it gets this far. Either way, crash if this // fails. - CHECK_EQ(fn(&session_, callbacks, this, *opts), 0); + CHECK_EQ(fn(&session_, callbacks, this, *opts, *allocator_info), 0); outgoing_storage_.reserve(4096); outgoing_buffers_.reserve(32); @@ -502,6 +590,7 @@ Http2Session::~Http2Session() { for (const auto& iter : streams_) iter.second->session_ = nullptr; nghttp2_session_del(session_); + CHECK_EQ(current_nghttp2_memory_, 0); } std::string Http2Session::diagnostic_name() const { @@ -1150,9 +1239,9 @@ void Http2Session::HandleHeadersFrame(const nghttp2_frame* frame) { nghttp2_header item = headers[n++]; // The header name and value are passed as external one-byte strings name_str = - ExternalHeader::New(env(), item.name).ToLocalChecked(); + ExternalHeader::New(this, item.name).ToLocalChecked(); value_str = - ExternalHeader::New(env(), item.value).ToLocalChecked(); + ExternalHeader::New(this, item.value).ToLocalChecked(); argv[j * 2] = name_str; argv[j * 2 + 1] = value_str; j++; diff --git a/src/node_http2.h b/src/node_http2.h index db453c1087378f..d90c3aed66b0a0 100644 --- a/src/node_http2.h +++ b/src/node_http2.h @@ -721,6 +721,7 @@ class Http2Session : public AsyncWrap, public StreamListener { class Http2Ping; class Http2Settings; + class MemoryAllocatorInfo; void EmitStatistics(); @@ -849,13 +850,15 @@ class Http2Session : public AsyncWrap, public StreamListener { current_session_memory_ -= amount; } - // Returns the current session memory including the current size of both - // the inflate and deflate hpack headers, the current outbound storage - // queue, and pending writes. + // Tell our custom memory allocator that this rcbuf is independent of + // this session now, and may outlive it. + void StopTrackingRcbuf(nghttp2_rcbuf* buf); + + // Returns the current session memory including memory allocated by nghttp2, + // the current outbound storage queue, and pending writes. uint64_t GetCurrentSessionMemory() { uint64_t total = current_session_memory_ + sizeof(Http2Session); - total += nghttp2_session_get_hd_deflate_dynamic_table_size(session_); - total += nghttp2_session_get_hd_inflate_dynamic_table_size(session_); + total += current_nghttp2_memory_; total += outgoing_storage_.size(); return total; } @@ -987,6 +990,8 @@ class Http2Session : public AsyncWrap, public StreamListener { // The maximum amount of memory allocated for this session uint64_t max_session_memory_ = DEFAULT_MAX_SESSION_MEMORY; uint64_t current_session_memory_ = 0; + // The amount of memory allocated by nghttp2 internals + uint64_t current_nghttp2_memory_ = 0; // The collection of active Http2Streams associated with this session std::unordered_map streams_; @@ -1178,7 +1183,8 @@ class ExternalHeader : } template - static MaybeLocal New(Environment* env, nghttp2_rcbuf* buf) { + static MaybeLocal New(Http2Session* session, nghttp2_rcbuf* buf) { + Environment* env = session->env(); if (nghttp2_rcbuf_is_static(buf)) { auto& static_str_map = env->isolate_data()->http2_static_strs; v8::Eternal& eternal = static_str_map[buf]; @@ -1205,6 +1211,7 @@ class ExternalHeader : return GetInternalizedString(env, vec); } + session->StopTrackingRcbuf(buf); ExternalHeader* h_str = new ExternalHeader(buf); MaybeLocal str = String::NewExternalOneByte(env->isolate(), h_str); if (str.IsEmpty()) diff --git a/src/util-inl.h b/src/util-inl.h index 90c7447f7e105f..c6cd263aa2714a 100644 --- a/src/util-inl.h +++ b/src/util-inl.h @@ -320,8 +320,9 @@ bool StringEqualNoCaseN(const char* a, const char* b, size_t length) { return true; } -inline size_t MultiplyWithOverflowCheck(size_t a, size_t b) { - size_t ret = a * b; +template +inline T MultiplyWithOverflowCheck(T a, T b) { + auto ret = a * b; if (a != 0) CHECK_EQ(b, ret / a); diff --git a/src/util.h b/src/util.h index fade27458f3e16..f7dcc5ea35abf8 100644 --- a/src/util.h +++ b/src/util.h @@ -67,6 +67,9 @@ inline char* Calloc(size_t n); inline char* UncheckedMalloc(size_t n); inline char* UncheckedCalloc(size_t n); +template +inline T MultiplyWithOverflowCheck(T a, T b); + // Used by the allocation functions when allocation fails. // Thin wrapper around v8::Isolate::LowMemoryNotification() that checks // whether V8 is initialized.