From 9be789e63219f4db2a2c492d83c2c5f675b7812c Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sat, 21 Sep 2019 18:57:46 +0200 Subject: [PATCH] http2: use shared memory tracking implementation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The extensive testing done on http2 makes it easier to make sure the implementation is correct (and doesn’t diverge unnecessarily). Refs: https://github.com/nodejs/quic/pull/126 Reviewed-By: Daniel Bevenius Reviewed-By: James M Snell PR-URL: https://github.com/nodejs/node/pull/30745 Refs: https://github.com/nodejs/quic/blob/34ee0bc96f804c73cb22b2945a1a78f780938492/src/node_mem.h Reviewed-By: Colin Ihrig Reviewed-By: Rich Trott --- src/node_http2.cc | 108 ++++++---------------------------------------- src/node_http2.h | 11 ++++- 2 files changed, 23 insertions(+), 96 deletions(-) diff --git a/src/node_http2.cc b/src/node_http2.cc index f78b6869bda688..1b5d52973f267c 100644 --- a/src/node_http2.cc +++ b/src/node_http2.cc @@ -5,6 +5,7 @@ #include "node_buffer.h" #include "node_http2.h" #include "node_http2_state.h" +#include "node_mem-inl.h" #include "node_perf.h" #include "node_revert.h" #include "util-inl.h" @@ -505,101 +506,20 @@ 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_NULL(result); - } - - 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. - // TODO(addaleax): Avoid the double bookkeeping we do with - // current_nghttp2_memory_ + AdjustAmountOfExternalAllocatedMemory - // and provide versions of our memory allocation utilities that take an - // Environment*/Isolate* parameter and call the V8 method transparently. - const int64_t new_size = size - previous_size; - session->current_nghttp2_memory_ += new_size; - session->env()->isolate()->AdjustAmountOfExternalAllocatedMemory( - new_size); - *reinterpret_cast(mem) = size; - mem += sizeof(size_t); - } else if (size == 0) { - session->current_nghttp2_memory_ -= previous_size; - session->env()->isolate()->AdjustAmountOfExternalAllocatedMemory( - -static_cast(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; - session->env()->isolate()->AdjustAmountOfExternalAllocatedMemory( - -static_cast(*original_ptr)); - *original_ptr = 0; - } +void Http2Session::StopTrackingRcbuf(nghttp2_rcbuf* buf) { + StopTrackingMemory(buf); +} - inline nghttp2_mem* operator*() { return &info; } +void Http2Session::CheckAllocatedSize(size_t previous_size) const { + CHECK_GE(current_nghttp2_memory_, previous_size); +} - nghttp2_mem info; -}; +void Http2Session::IncreaseAllocatedSize(size_t size) { + current_nghttp2_memory_ += size; +} -void Http2Session::StopTrackingRcbuf(nghttp2_rcbuf* buf) { - MemoryAllocatorInfo::StopTracking(this, buf); +void Http2Session::DecreaseAllocatedSize(size_t size) { + current_nghttp2_memory_ -= size; } Http2Session::Http2Session(Environment* env, @@ -636,14 +556,14 @@ Http2Session::Http2Session(Environment* env, nghttp2_session_server_new3 : nghttp2_session_client_new3; - MemoryAllocatorInfo allocator_info(this); + nghttp2_mem alloc_info = MakeAllocator(); // 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, *allocator_info), 0); + CHECK_EQ(fn(&session_, callbacks, this, *opts, &alloc_info), 0); outgoing_storage_.reserve(1024); outgoing_buffers_.reserve(32); diff --git a/src/node_http2.h b/src/node_http2.h index 518a0e6c1af4ed..c70d3f5c2b6347 100644 --- a/src/node_http2.h +++ b/src/node_http2.h @@ -8,6 +8,7 @@ #include "nghttp2/nghttp2.h" #include "node_http2_state.h" +#include "node_mem.h" #include "node_perf.h" #include "stream_base-inl.h" #include "string_bytes.h" @@ -705,7 +706,9 @@ enum SessionBitfieldFlags { kSessionHasAltsvcListeners }; -class Http2Session : public AsyncWrap, public StreamListener { +class Http2Session : public AsyncWrap, + public StreamListener, + public mem::NgLibMemoryManager { public: Http2Session(Environment* env, Local wrap, @@ -714,7 +717,6 @@ class Http2Session : public AsyncWrap, public StreamListener { class Http2Ping; class Http2Settings; - class MemoryAllocatorInfo; void EmitStatistics(); @@ -815,6 +817,11 @@ class Http2Session : public AsyncWrap, public StreamListener { void OnStreamRead(ssize_t nread, const uv_buf_t& buf) override; void OnStreamAfterWrite(WriteWrap* w, int status) override; + // Implementation for mem::NgLibMemoryManager + void CheckAllocatedSize(size_t previous_size) const; + void IncreaseAllocatedSize(size_t size); + void DecreaseAllocatedSize(size_t size); + // The JavaScript API static void New(const FunctionCallbackInfo& args); static void Consume(const FunctionCallbackInfo& args);