Skip to content

Commit

Permalink
http2: use shared memory tracking implementation
Browse files Browse the repository at this point in the history
The extensive testing done on http2 makes it easier to make sure
the implementation is correct (and doesn’t diverge unnecessarily).

Refs: nodejs/quic#126
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

PR-URL: #30745
Refs: https://github.com/nodejs/quic/blob/34ee0bc96f804c73cb22b2945a1a78f780938492/src/node_mem.h
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
  • Loading branch information
addaleax authored and targos committed Jan 14, 2020
1 parent b1c2264 commit 284f033
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 96 deletions.
108 changes: 14 additions & 94 deletions src/node_http2.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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<Http2Session*>(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<char*>(ptr) - sizeof(size_t);
previous_size = *reinterpret_cast<size_t*>(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<size_t*>(mem) = size;
mem += sizeof(size_t);
} else if (size == 0) {
session->current_nghttp2_memory_ -= previous_size;
session->env()->isolate()->AdjustAmountOfExternalAllocatedMemory(
-static_cast<int64_t>(previous_size));
}

return mem;
}

static void StopTracking(Http2Session* session, void* ptr) {
size_t* original_ptr = reinterpret_cast<size_t*>(
static_cast<char*>(ptr) - sizeof(size_t));
session->current_nghttp2_memory_ -= *original_ptr;
session->env()->isolate()->AdjustAmountOfExternalAllocatedMemory(
-static_cast<int64_t>(*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,
Expand Down Expand Up @@ -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);
Expand Down
11 changes: 9 additions & 2 deletions src/node_http2.h
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -705,7 +706,9 @@ enum SessionBitfieldFlags {
kSessionHasAltsvcListeners
};

class Http2Session : public AsyncWrap, public StreamListener {
class Http2Session : public AsyncWrap,
public StreamListener,
public mem::NgLibMemoryManager<Http2Session, nghttp2_mem> {
public:
Http2Session(Environment* env,
Local<Object> wrap,
Expand All @@ -714,7 +717,6 @@ class Http2Session : public AsyncWrap, public StreamListener {

class Http2Ping;
class Http2Settings;
class MemoryAllocatorInfo;

void EmitStatistics();

Expand Down Expand Up @@ -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<Value>& args);
static void Consume(const FunctionCallbackInfo<Value>& args);
Expand Down

0 comments on commit 284f033

Please sign in to comment.