diff --git a/doc/api/errors.md b/doc/api/errors.md index 6a286772dee..a90080e79bc 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -613,6 +613,19 @@ An attempt was made to register something that is not a function as an The type of an asynchronous resource was invalid. Note that users are also able to define their own types if using the public embedder API. + +### ERR_BUFFER_CONTEXT_NOT_AVAILABLE + +An attempt was made to create a Node.js `Buffer` instance from addon or embedder +code, while in a JS engine Context that is not associated with a Node.js +instance. The data passed to the `Buffer` method will have been released +by the time the method returns. + +When encountering this error, a possible alternative to creating a `Buffer` +instance is to create a normal `Uint8Array`, which only differs in the +prototype of the resulting object. `Uint8Array`s are generally accepted in all +Node.js core APIs where `Buffer`s are; they are available in all Contexts. + ### ERR_BUFFER_OUT_OF_BOUNDS diff --git a/src/env-inl.h b/src/env-inl.h index 4e0a95ed424..bff754f5a0e 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -47,8 +47,16 @@ inline uv_loop_t* IsolateData::event_loop() const { return event_loop_; } -inline uint32_t* IsolateData::zero_fill_field() const { - return zero_fill_field_; +inline bool IsolateData::uses_node_allocator() const { + return uses_node_allocator_; +} + +inline v8::ArrayBuffer::Allocator* IsolateData::allocator() const { + return allocator_; +} + +inline ArrayBufferAllocator* IsolateData::node_allocator() const { + return node_allocator_; } inline MultiIsolatePlatform* IsolateData::platform() const { @@ -661,6 +669,104 @@ inline IsolateData* Environment::isolate_data() const { return isolate_data_; } +inline char* Environment::AllocateUnchecked(size_t size) { + return static_cast( + isolate_data()->allocator()->AllocateUninitialized(size)); +} + +inline char* Environment::Allocate(size_t size) { + char* ret = AllocateUnchecked(size); + CHECK_NE(ret, nullptr); + return ret; +} + +inline void Environment::Free(char* data, size_t size) { + if (data != nullptr) + isolate_data()->allocator()->Free(data, size); +} + +inline AllocatedBuffer Environment::AllocateManaged(size_t size, bool checked) { + char* data = checked ? Allocate(size) : AllocateUnchecked(size); + if (data == nullptr) size = 0; + return AllocatedBuffer(this, uv_buf_init(data, size)); +} + +inline AllocatedBuffer::AllocatedBuffer(Environment* env, uv_buf_t buf) + : env_(env), buffer_(buf) {} + +inline void AllocatedBuffer::Resize(size_t len) { + char* new_data = env_->Reallocate(buffer_.base, buffer_.len, len); + CHECK_IMPLIES(len > 0, new_data != nullptr); + buffer_ = uv_buf_init(new_data, len); +} + +inline uv_buf_t AllocatedBuffer::release() { + uv_buf_t ret = buffer_; + buffer_ = uv_buf_init(nullptr, 0); + return ret; +} + +inline char* AllocatedBuffer::data() { + return buffer_.base; +} + +inline const char* AllocatedBuffer::data() const { + return buffer_.base; +} + +inline size_t AllocatedBuffer::size() const { + return buffer_.len; +} + +inline AllocatedBuffer::AllocatedBuffer(Environment* env) + : env_(env), buffer_(uv_buf_init(nullptr, 0)) {} + +inline AllocatedBuffer::AllocatedBuffer(AllocatedBuffer&& other) + : AllocatedBuffer() { + *this = std::move(other); +} + +inline AllocatedBuffer& AllocatedBuffer::operator=(AllocatedBuffer&& other) { + clear(); + env_ = other.env_; + buffer_ = other.release(); + return *this; +} + +inline AllocatedBuffer::~AllocatedBuffer() { + clear(); +} + +inline void AllocatedBuffer::clear() { + uv_buf_t buf = release(); + env_->Free(buf.base, buf.len); +} + +// It's a bit awkward to define this Buffer::New() overload here, but it +// avoids a circular dependency with node_internals.h. +namespace Buffer { +v8::MaybeLocal New(Environment* env, + char* data, + size_t length, + bool uses_malloc); +} + +inline v8::MaybeLocal AllocatedBuffer::ToBuffer() { + CHECK_NOT_NULL(env_); + v8::MaybeLocal obj = Buffer::New(env_, data(), size(), false); + if (!obj.IsEmpty()) release(); + return obj; +} + +inline v8::Local AllocatedBuffer::ToArrayBuffer() { + CHECK_NOT_NULL(env_); + uv_buf_t buf = release(); + return v8::ArrayBuffer::New(env_->isolate(), + buf.base, + buf.len, + v8::ArrayBufferCreationMode::kInternalized); +} + inline void Environment::ThrowError(const char* errmsg) { ThrowError(v8::Exception::Error, errmsg); } diff --git a/src/env.cc b/src/env.cc index d76e691a53f..47cd43e8394 100644 --- a/src/env.cc +++ b/src/env.cc @@ -13,6 +13,7 @@ namespace node { +using v8::ArrayBuffer; using v8::Context; using v8::FunctionTemplate; using v8::HandleScope; @@ -36,11 +37,14 @@ void* Environment::kNodeContextTagPtr = const_cast( IsolateData::IsolateData(Isolate* isolate, uv_loop_t* event_loop, MultiIsolatePlatform* platform, - uint32_t* zero_fill_field) : - isolate_(isolate), - event_loop_(event_loop), - zero_fill_field_(zero_fill_field), - platform_(platform) { + ArrayBufferAllocator* node_allocator) + : isolate_(isolate), + event_loop_(event_loop), + allocator_(isolate->GetArrayBufferAllocator()), + node_allocator_(node_allocator), + uses_node_allocator_(allocator_ == node_allocator_), + platform_(platform) { + CHECK_NOT_NULL(allocator_); if (platform_ != nullptr) platform_->RegisterIsolate(isolate_, event_loop); @@ -688,6 +692,23 @@ void Environment::BuildEmbedderGraph(v8::Isolate* isolate, }); } +char* Environment::Reallocate(char* data, size_t old_size, size_t size) { + // If we know that the allocator is our ArrayBufferAllocator, we can let + // if reallocate directly. + if (isolate_data()->uses_node_allocator()) { + return static_cast( + isolate_data()->node_allocator()->Reallocate(data, old_size, size)); + } + // Generic allocators do not provide a reallocation method; we need to + // allocate a new chunk of memory and copy the data over. + char* new_data = AllocateUnchecked(size); + if (new_data == nullptr) return nullptr; + memcpy(new_data, data, std::min(size, old_size)); + if (size > old_size) + memset(new_data + old_size, 0, size - old_size); + Free(data, old_size); + return new_data; +} // Not really any better place than env.cc at this moment. void BaseObject::DeleteMe(void* data) { diff --git a/src/env.h b/src/env.h index 4b678cee289..e0ef958dbcd 100644 --- a/src/env.h +++ b/src/env.h @@ -362,15 +362,19 @@ class Environment; class IsolateData { public: - IsolateData(v8::Isolate* isolate, uv_loop_t* event_loop, + IsolateData(v8::Isolate* isolate, + uv_loop_t* event_loop, MultiIsolatePlatform* platform = nullptr, - uint32_t* zero_fill_field = nullptr); + ArrayBufferAllocator* node_allocator = nullptr); ~IsolateData(); inline uv_loop_t* event_loop() const; - inline uint32_t* zero_fill_field() const; inline MultiIsolatePlatform* platform() const; inline std::shared_ptr options(); + inline bool uses_node_allocator() const; + inline v8::ArrayBuffer::Allocator* allocator() const; + inline ArrayBufferAllocator* node_allocator() const; + #define VP(PropertyName, StringValue) V(v8::Private, PropertyName) #define VY(PropertyName, StringValue) V(v8::Symbol, PropertyName) #define VS(PropertyName, StringValue) V(v8::String, PropertyName) @@ -401,7 +405,9 @@ class IsolateData { v8::Isolate* const isolate_; uv_loop_t* const event_loop_; - uint32_t* const zero_fill_field_; + v8::ArrayBuffer::Allocator* const allocator_; + ArrayBufferAllocator* const node_allocator_; + const bool uses_node_allocator_; MultiIsolatePlatform* platform_; std::shared_ptr options_; @@ -428,6 +434,38 @@ enum class DebugCategory { CATEGORY_COUNT }; +// A unique-pointer-ish object that is compatible with the JS engine's +// ArrayBuffer::Allocator. +struct AllocatedBuffer { + public: + explicit inline AllocatedBuffer(Environment* env = nullptr); + inline AllocatedBuffer(Environment* env, uv_buf_t buf); + inline ~AllocatedBuffer(); + inline void Resize(size_t len); + + inline uv_buf_t release(); + inline char* data(); + inline const char* data() const; + inline size_t size() const; + inline void clear(); + + inline v8::MaybeLocal ToBuffer(); + inline v8::Local ToArrayBuffer(); + + inline AllocatedBuffer(AllocatedBuffer&& other); + inline AllocatedBuffer& operator=(AllocatedBuffer&& other); + AllocatedBuffer(const AllocatedBuffer& other) = delete; + AllocatedBuffer& operator=(const AllocatedBuffer& other) = delete; + + private: + Environment* env_; + // We do not pass this to libuv directly, but uv_buf_t is a convenient way + // to represent a chunk of memory, and plays nicely with other parts of core. + uv_buf_t buffer_; + + friend class Environment; +}; + class Environment { public: class AsyncHooks { @@ -642,6 +680,15 @@ class Environment { inline IsolateData* isolate_data() const; + // Utilites that allocate memory using the Isolate's ArrayBuffer::Allocator. + // In particular, using AllocateManaged() will provide a RAII-style object + // with easy conversion to `Buffer` and `ArrayBuffer` objects. + inline AllocatedBuffer AllocateManaged(size_t size, bool checked = true); + inline char* Allocate(size_t size); + inline char* AllocateUnchecked(size_t size); + char* Reallocate(char* data, size_t old_size, size_t size); + inline void Free(char* data, size_t size); + inline bool printed_error() const; inline void set_printed_error(bool value); diff --git a/src/node.cc b/src/node.cc index aa51c0b46a1..21a9c9beefa 100644 --- a/src/node.cc +++ b/src/node.cc @@ -629,6 +629,77 @@ void* ArrayBufferAllocator::Allocate(size_t size) { return UncheckedMalloc(size); } +DebuggingArrayBufferAllocator::~DebuggingArrayBufferAllocator() { + CHECK(allocations_.empty()); +} + +void* DebuggingArrayBufferAllocator::Allocate(size_t size) { + Mutex::ScopedLock lock(mutex_); + void* data = ArrayBufferAllocator::Allocate(size); + RegisterPointerInternal(data, size); + return data; +} + +void* DebuggingArrayBufferAllocator::AllocateUninitialized(size_t size) { + Mutex::ScopedLock lock(mutex_); + void* data = ArrayBufferAllocator::AllocateUninitialized(size); + RegisterPointerInternal(data, size); + return data; +} + +void DebuggingArrayBufferAllocator::Free(void* data, size_t size) { + Mutex::ScopedLock lock(mutex_); + UnregisterPointerInternal(data, size); + ArrayBufferAllocator::Free(data, size); +} + +void* DebuggingArrayBufferAllocator::Reallocate(void* data, + size_t old_size, + size_t size) { + Mutex::ScopedLock lock(mutex_); + void* ret = ArrayBufferAllocator::Reallocate(data, old_size, size); + if (ret == nullptr) { + if (size == 0) // i.e. equivalent to free(). + UnregisterPointerInternal(data, old_size); + return nullptr; + } + + if (data != nullptr) { + auto it = allocations_.find(data); + CHECK_NE(it, allocations_.end()); + allocations_.erase(it); + } + + RegisterPointerInternal(ret, size); + return ret; +} + +void DebuggingArrayBufferAllocator::RegisterPointer(void* data, size_t size) { + Mutex::ScopedLock lock(mutex_); + RegisterPointerInternal(data, size); +} + +void DebuggingArrayBufferAllocator::UnregisterPointer(void* data, size_t size) { + Mutex::ScopedLock lock(mutex_); + UnregisterPointerInternal(data, size); +} + +void DebuggingArrayBufferAllocator::UnregisterPointerInternal(void* data, + size_t size) { + if (data == nullptr) return; + auto it = allocations_.find(data); + CHECK_NE(it, allocations_.end()); + CHECK_EQ(it->second, size); + allocations_.erase(it); +} + +void DebuggingArrayBufferAllocator::RegisterPointerInternal(void* data, + size_t size) { + if (data == nullptr) return; + CHECK_EQ(allocations_.count(data), 0); + allocations_[data] = size; +} + namespace { bool ShouldAbortOnUncaughtException(Isolate* isolate) { @@ -2805,7 +2876,10 @@ int EmitExit(Environment* env) { ArrayBufferAllocator* CreateArrayBufferAllocator() { - return new ArrayBufferAllocator(); + if (per_process_opts->debug_arraybuffer_allocations) + return new DebuggingArrayBufferAllocator(); + else + return new ArrayBufferAllocator(); } @@ -2832,7 +2906,7 @@ IsolateData* CreateIsolateData( uv_loop_t* loop, MultiIsolatePlatform* platform, ArrayBufferAllocator* allocator) { - return new IsolateData(isolate, loop, platform, allocator->zero_fill_field()); + return new IsolateData(isolate, loop, platform, allocator); } diff --git a/src/node_buffer.cc b/src/node_buffer.cc index 475770def92..94f978f89af 100644 --- a/src/node_buffer.cc +++ b/src/node_buffer.cc @@ -55,20 +55,6 @@ size_t length = end - start; namespace node { - -// if true, all Buffer and SlowBuffer instances will automatically zero-fill -bool zero_fill_all_buffers = false; - -namespace { - -inline void* BufferMalloc(v8::Isolate* isolate, size_t length) { - auto* allocator = isolate->GetArrayBufferAllocator(); - return zero_fill_all_buffers ? allocator->Allocate(length) : - allocator->AllocateUninitialized(length); -} - -} // namespace - namespace Buffer { using v8::ArrayBuffer; @@ -246,7 +232,7 @@ MaybeLocal New(Isolate* isolate, char* data = nullptr; if (length > 0) { - data = static_cast(BufferMalloc(isolate, length)); + data = UncheckedMalloc(length); if (data == nullptr) return Local(); @@ -255,24 +241,14 @@ MaybeLocal New(Isolate* isolate, CHECK(actual <= length); if (actual == 0) { - isolate->GetArrayBufferAllocator()->Free(data, length); + free(data); data = nullptr; } else if (actual < length) { - auto* allocator = isolate->GetArrayBufferAllocator(); - auto* excessive_data = data; - data = static_cast(allocator->AllocateUninitialized(actual)); - memcpy(data, excessive_data, actual); - allocator->Free(excessive_data, length); + data = node::Realloc(data, actual); } } - Local buf; - if (New(isolate, data, actual).ToLocal(&buf)) - return scope.Escape(buf); - - // Object failed to be created. Clean up resources. - isolate->GetArrayBufferAllocator()->Free(data, length); - return Local(); + return scope.EscapeMaybe(New(isolate, data, actual)); } @@ -280,7 +256,10 @@ MaybeLocal New(Isolate* isolate, size_t length) { EscapableHandleScope handle_scope(isolate); Local obj; Environment* env = Environment::GetCurrent(isolate); - CHECK_NOT_NULL(env); // TODO(addaleax): Handle nullptr here. + if (env == nullptr) { + THROW_ERR_BUFFER_CONTEXT_NOT_AVAILABLE(isolate); + return MaybeLocal(); + } if (Buffer::New(env, length).ToLocal(&obj)) return handle_scope.Escape(obj); return Local(); @@ -295,35 +274,26 @@ MaybeLocal New(Environment* env, size_t length) { return Local(); } - void* data; + AllocatedBuffer ret(env); if (length > 0) { - data = BufferMalloc(env->isolate(), length); - if (data == nullptr) + ret = env->AllocateManaged(length, false); + if (ret.data() == nullptr) { + THROW_ERR_MEMORY_ALLOCATION_FAILED(env); return Local(); - } else { - data = nullptr; - } - - Local ab = - ArrayBuffer::New(env->isolate(), - data, - length, - ArrayBufferCreationMode::kInternalized); - MaybeLocal ui = Buffer::New(env, ab, 0, length); - - if (ui.IsEmpty()) { - // Object failed to be created. Clean up resources. - env->isolate()->GetArrayBufferAllocator()->Free(data, length); + } } - return scope.Escape(ui.FromMaybe(Local())); + return scope.EscapeMaybe(ret.ToBuffer()); } MaybeLocal Copy(Isolate* isolate, const char* data, size_t length) { EscapableHandleScope handle_scope(isolate); Environment* env = Environment::GetCurrent(isolate); - CHECK_NOT_NULL(env); // TODO(addaleax): Handle nullptr here. + if (env == nullptr) { + THROW_ERR_BUFFER_CONTEXT_NOT_AVAILABLE(isolate); + return MaybeLocal(); + } Local obj; if (Buffer::Copy(env, data, length).ToLocal(&obj)) return handle_scope.Escape(obj); @@ -339,31 +309,18 @@ MaybeLocal Copy(Environment* env, const char* data, size_t length) { return Local(); } - auto* allocator = env->isolate()->GetArrayBufferAllocator(); - void* new_data; + AllocatedBuffer ret(env); if (length > 0) { CHECK_NOT_NULL(data); - new_data = allocator->AllocateUninitialized(length); - if (new_data == nullptr) + ret = env->AllocateManaged(length, false); + if (ret.data() == nullptr) { + THROW_ERR_MEMORY_ALLOCATION_FAILED(env); return Local(); - memcpy(new_data, data, length); - } else { - new_data = nullptr; - } - - Local ab = - ArrayBuffer::New(env->isolate(), - new_data, - length, - ArrayBufferCreationMode::kInternalized); - MaybeLocal ui = Buffer::New(env, ab, 0, length); - - if (ui.IsEmpty()) { - // Object failed to be created. Clean up resources. - allocator->Free(new_data, length); + } + memcpy(ret.data(), data, length); } - return scope.Escape(ui.FromMaybe(Local())); + return scope.EscapeMaybe(ret.ToBuffer()); } @@ -374,7 +331,11 @@ MaybeLocal New(Isolate* isolate, void* hint) { EscapableHandleScope handle_scope(isolate); Environment* env = Environment::GetCurrent(isolate); - CHECK_NOT_NULL(env); // TODO(addaleax): Handle nullptr here. + if (env == nullptr) { + callback(data, hint); + THROW_ERR_BUFFER_CONTEXT_NOT_AVAILABLE(isolate); + return MaybeLocal(); + } Local obj; if (Buffer::New(env, data, length, callback, hint).ToLocal(&obj)) return handle_scope.Escape(obj); @@ -394,11 +355,6 @@ MaybeLocal New(Environment* env, } Local ab = ArrayBuffer::New(env->isolate(), data, length); - // `Neuter()`ing is required here to prevent materialization of the backing - // store in v8. `nullptr` buffers are not writable, so this is semantically - // correct. - if (data == nullptr) - ab->Neuter(); MaybeLocal ui = Buffer::New(env, ab, 0, length); if (ui.IsEmpty()) { @@ -409,24 +365,48 @@ MaybeLocal New(Environment* env, return scope.Escape(ui.ToLocalChecked()); } - +// Warning: This function needs `data` to be allocated with malloc() and not +// necessarily isolate's ArrayBuffer::Allocator. MaybeLocal New(Isolate* isolate, char* data, size_t length) { EscapableHandleScope handle_scope(isolate); Environment* env = Environment::GetCurrent(isolate); - CHECK_NOT_NULL(env); // TODO(addaleax): Handle nullptr here. + if (env == nullptr) { + free(data); + THROW_ERR_BUFFER_CONTEXT_NOT_AVAILABLE(isolate); + return MaybeLocal(); + } Local obj; - if (Buffer::New(env, data, length).ToLocal(&obj)) + if (Buffer::New(env, data, length, true).ToLocal(&obj)) return handle_scope.Escape(obj); return Local(); } - -MaybeLocal New(Environment* env, char* data, size_t length) { +// Warning: If this call comes through the public node_buffer.h API, +// the contract for this function is that `data` is allocated with malloc() +// and not necessarily isolate's ArrayBuffer::Allocator. +MaybeLocal New(Environment* env, + char* data, + size_t length, + bool uses_malloc) { if (length > 0) { CHECK_NOT_NULL(data); CHECK(length <= kMaxLength); } + if (uses_malloc) { + if (!env->isolate_data()->uses_node_allocator()) { + // We don't know for sure that the allocator is malloc()-based, so we need + // to fall back to the FreeCallback variant. + auto free_callback = [](char* data, void* hint) { free(data); }; + return New(env, data, length, free_callback, nullptr); + } else { + // This is malloc()-based, so we can acquire it into our own + // ArrayBufferAllocator. + CHECK_NOT_NULL(env->isolate_data()->node_allocator()); + env->isolate_data()->node_allocator()->RegisterPointer(data, length); + } + } + Local ab = ArrayBuffer::New(env->isolate(), data, @@ -1026,15 +1006,13 @@ static void EncodeUtf8String(const FunctionCallbackInfo& args) { Local str = args[0].As(); size_t length = str->Utf8Length(isolate); - char* data = node::UncheckedMalloc(length); + AllocatedBuffer buf = env->AllocateManaged(length); str->WriteUtf8(isolate, - data, + buf.data(), -1, // We are certain that `data` is sufficiently large nullptr, String::NO_NULL_TERMINATION | String::REPLACE_INVALID_UTF8); - auto array_buf = ArrayBuffer::New( - isolate, data, length, ArrayBufferCreationMode::kInternalized); - auto array = Uint8Array::New(array_buf, 0, length); + auto array = Uint8Array::New(buf.ToArrayBuffer(), 0, length); args.GetReturnValue().Set(array); } @@ -1061,7 +1039,8 @@ void SetupBufferJS(const FunctionCallbackInfo& args) { env->SetMethod(proto, "ucs2Write", StringWrite); env->SetMethod(proto, "utf8Write", StringWrite); - if (auto zero_fill_field = env->isolate_data()->zero_fill_field()) { + if (ArrayBufferAllocator* allocator = env->isolate_data()->node_allocator()) { + uint32_t* zero_fill_field = allocator->zero_fill_field(); CHECK(args[1]->IsObject()); auto binding_object = args[1].As(); auto array_buffer = ArrayBuffer::New(env->isolate(), diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 21ca17c13f8..0194c8c5bbb 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -323,6 +323,14 @@ bool EntropySource(unsigned char* buffer, size_t length) { } +template +static T* MallocOpenSSL(size_t count) { + void* mem = OPENSSL_malloc(MultiplyWithOverflowCheck(count, sizeof(T))); + CHECK_IMPLIES(mem == nullptr, count == 0); + return static_cast(mem); +} + + void SecureContext::Initialize(Environment* env, Local target) { Local t = env->NewFunctionTemplate(New); t->InstanceTemplate()->SetInternalFieldCount(1); @@ -1556,6 +1564,30 @@ static void AddFingerprintDigest(const unsigned char* md, } } +static MaybeLocal ECPointToBuffer(Environment* env, + const EC_GROUP* group, + const EC_POINT* point, + point_conversion_form_t form, + const char** error) { + size_t len = EC_POINT_point2oct(group, point, form, nullptr, 0, nullptr); + if (len == 0) { + if (error != nullptr) *error = "Failed to get public key length"; + return MaybeLocal(); + } + AllocatedBuffer buf = env->AllocateManaged(len); + len = EC_POINT_point2oct(group, + point, + form, + reinterpret_cast(buf.data()), + buf.size(), + nullptr); + if (len == 0) { + if (error != nullptr) *error = "Failed to get public key"; + return MaybeLocal(); + } + return buf.ToBuffer(); +} + static Local X509ToObject(Environment* env, X509* cert) { EscapableHandleScope scope(env->isolate()); Local context = env->context(); @@ -1867,10 +1899,9 @@ void SSLWrap::GetFinished(const FunctionCallbackInfo& args) { if (len == 0) return; - auto* allocator = env->isolate()->GetArrayBufferAllocator(); - char* buf = static_cast(allocator->AllocateUninitialized(len)); - CHECK_EQ(len, SSL_get_finished(w->ssl_.get(), buf, len)); - args.GetReturnValue().Set(Buffer::New(env, buf, len).ToLocalChecked()); + AllocatedBuffer buf = env->AllocateManaged(len); + CHECK_EQ(len, SSL_get_finished(w->ssl_.get(), buf.data(), len)); + args.GetReturnValue().Set(buf.ToBuffer().ToLocalChecked()); } @@ -1891,10 +1922,9 @@ void SSLWrap::GetPeerFinished(const FunctionCallbackInfo& args) { if (len == 0) return; - auto* allocator = env->isolate()->GetArrayBufferAllocator(); - char* buf = static_cast(allocator->AllocateUninitialized(len)); - CHECK_EQ(len, SSL_get_peer_finished(w->ssl_.get(), buf, len)); - args.GetReturnValue().Set(Buffer::New(env, buf, len).ToLocalChecked()); + AllocatedBuffer buf = env->AllocateManaged(len); + CHECK_EQ(len, SSL_get_peer_finished(w->ssl_.get(), buf.data(), len)); + args.GetReturnValue().Set(buf.ToBuffer().ToLocalChecked()); } @@ -1912,11 +1942,10 @@ void SSLWrap::GetSession(const FunctionCallbackInfo& args) { int slen = i2d_SSL_SESSION(sess, nullptr); CHECK_GT(slen, 0); - auto* allocator = env->isolate()->GetArrayBufferAllocator(); - char* sbuf = static_cast(allocator->AllocateUninitialized(slen)); - unsigned char* p = reinterpret_cast(sbuf); + AllocatedBuffer sbuf = env->AllocateManaged(slen); + unsigned char* p = reinterpret_cast(sbuf.data()); i2d_SSL_SESSION(sess, &p); - args.GetReturnValue().Set(Buffer::New(env, sbuf, slen).ToLocalChecked()); + args.GetReturnValue().Set(sbuf.ToBuffer().ToLocalChecked()); } @@ -2334,12 +2363,11 @@ int SSLWrap::TLSExtStatusCallback(SSL* s, void* arg) { size_t len = Buffer::Length(obj); // OpenSSL takes control of the pointer after accepting it - auto* allocator = env->isolate()->GetArrayBufferAllocator(); - uint8_t* data = static_cast(allocator->AllocateUninitialized(len)); + unsigned char* data = MallocOpenSSL(len); memcpy(data, resp, len); if (!SSL_set_tlsext_status_ocsp_resp(s, data, len)) - allocator->Free(data, len); + OPENSSL_free(data); w->ocsp_response_.Reset(); return SSL_TLSEXT_ERR_OK; @@ -3019,11 +3047,9 @@ void CipherBase::SetAAD(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(b); // Possibly report invalid state failure } - CipherBase::UpdateResult CipherBase::Update(const char* data, int len, - unsigned char** out, - int* out_len) { + AllocatedBuffer* out) { if (!ctx_) return kErrorState; MarkPopErrorOnReturn mark_pop_error_on_return; @@ -3041,28 +3067,27 @@ CipherBase::UpdateResult CipherBase::Update(const char* data, CHECK(MaybePassAuthTagToOpenSSL()); } - *out_len = 0; - int buff_len = len + EVP_CIPHER_CTX_block_size(ctx_.get()); + int buf_len = len + EVP_CIPHER_CTX_block_size(ctx_.get()); // For key wrapping algorithms, get output size by calling // EVP_CipherUpdate() with null output. if (kind_ == kCipher && mode == EVP_CIPH_WRAP_MODE && EVP_CipherUpdate(ctx_.get(), nullptr, - &buff_len, + &buf_len, reinterpret_cast(data), len) != 1) { return kErrorState; } - auto* allocator = env()->isolate()->GetArrayBufferAllocator(); - *out = static_cast(allocator->AllocateUninitialized(buff_len)); + *out = env()->AllocateManaged(buf_len); int r = EVP_CipherUpdate(ctx_.get(), - *out, - out_len, + reinterpret_cast(out->data()), + &buf_len, reinterpret_cast(data), len); - CHECK_LE(*out_len, buff_len); + CHECK_LE(static_cast(buf_len), out->size()); + out->Resize(buf_len); // When in CCM mode, EVP_CipherUpdate will fail if the authentication tag is // invalid. In that case, remember the error and throw in final(). @@ -3080,9 +3105,8 @@ void CipherBase::Update(const FunctionCallbackInfo& args) { CipherBase* cipher; ASSIGN_OR_RETURN_UNWRAP(&cipher, args.Holder()); - unsigned char* out = nullptr; + AllocatedBuffer out; UpdateResult r; - int out_len = 0; // Only copy the data if we have to, because it's a string if (args[0]->IsString()) { @@ -3090,16 +3114,14 @@ void CipherBase::Update(const FunctionCallbackInfo& args) { if (!decoder.Decode(env, args[0].As(), args[1], UTF8) .FromMaybe(false)) return; - r = cipher->Update(decoder.out(), decoder.size(), &out, &out_len); + r = cipher->Update(decoder.out(), decoder.size(), &out); } else { char* buf = Buffer::Data(args[0]); size_t buflen = Buffer::Length(args[0]); - r = cipher->Update(buf, buflen, &out, &out_len); + r = cipher->Update(buf, buflen, &out); } if (r != kSuccess) { - auto* allocator = env->isolate()->GetArrayBufferAllocator(); - allocator->Free(out, out_len); if (r == kErrorState) { ThrowCryptoError(env, ERR_get_error(), "Trying to add data in unsupported state"); @@ -3107,11 +3129,9 @@ void CipherBase::Update(const FunctionCallbackInfo& args) { return; } - CHECK(out != nullptr || out_len == 0); - Local buf = - Buffer::New(env, reinterpret_cast(out), out_len).ToLocalChecked(); + CHECK(out.data() != nullptr || out.size() == 0); - args.GetReturnValue().Set(buf); + args.GetReturnValue().Set(out.ToBuffer().ToLocalChecked()); } @@ -3131,16 +3151,14 @@ void CipherBase::SetAutoPadding(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(b); // Possibly report invalid state failure } - -bool CipherBase::Final(unsigned char** out, int* out_len) { +bool CipherBase::Final(AllocatedBuffer* out) { if (!ctx_) return false; const int mode = EVP_CIPHER_CTX_mode(ctx_.get()); - auto* allocator = env()->isolate()->GetArrayBufferAllocator(); - *out = static_cast(allocator->AllocateUninitialized( - EVP_CIPHER_CTX_block_size(ctx_.get()))); + *out = env()->AllocateManaged( + static_cast(EVP_CIPHER_CTX_block_size(ctx_.get()))); if (kind_ == kDecipher && IsSupportedAuthenticatedMode(mode)) { MaybePassAuthTagToOpenSSL(); @@ -3151,8 +3169,17 @@ bool CipherBase::Final(unsigned char** out, int* out_len) { bool ok; if (kind_ == kDecipher && mode == EVP_CIPH_CCM_MODE) { ok = !pending_auth_failed_; + *out = AllocatedBuffer(env()); // Empty buffer. } else { - ok = EVP_CipherFinal_ex(ctx_.get(), *out, out_len) == 1; + int out_len = out->size(); + ok = EVP_CipherFinal_ex(ctx_.get(), + reinterpret_cast(out->data()), + &out_len) == 1; + + if (out_len >= 0) + out->Resize(out_len); + else + *out = AllocatedBuffer(); // *out will not be used. if (ok && kind_ == kCipher && IsAuthenticatedMode()) { // In GCM mode, the authentication tag length can be specified in advance, @@ -3181,34 +3208,21 @@ void CipherBase::Final(const FunctionCallbackInfo& args) { ASSIGN_OR_RETURN_UNWRAP(&cipher, args.Holder()); if (cipher->ctx_ == nullptr) return env->ThrowError("Unsupported state"); - unsigned char* out_value = nullptr; - int out_len = -1; + AllocatedBuffer out; // Check IsAuthenticatedMode() first, Final() destroys the EVP_CIPHER_CTX. const bool is_auth_mode = cipher->IsAuthenticatedMode(); - bool r = cipher->Final(&out_value, &out_len); - - if (out_len <= 0 || !r) { - auto* allocator = env->isolate()->GetArrayBufferAllocator(); - allocator->Free(out_value, out_len); - out_value = nullptr; - out_len = 0; - if (!r) { - const char* msg = is_auth_mode ? - "Unsupported state or unable to authenticate data" : - "Unsupported state"; - - return ThrowCryptoError(env, - ERR_get_error(), - msg); - } + bool r = cipher->Final(&out); + + if (!r) { + const char* msg = is_auth_mode + ? "Unsupported state or unable to authenticate data" + : "Unsupported state"; + + return ThrowCryptoError(env, ERR_get_error(), msg); } - Local buf = Buffer::New( - env, - reinterpret_cast(out_value), - out_len).ToLocalChecked(); - args.GetReturnValue().Set(buf); + args.GetReturnValue().Set(out.ToBuffer().ToLocalChecked()); } @@ -3835,7 +3849,6 @@ void Verify::VerifyFinal(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(verify_result); } - template @@ -3846,10 +3859,8 @@ bool PublicKeyCipher::Cipher(Environment* env, int padding, const unsigned char* data, int len, - unsigned char** out, - size_t* out_len) { + AllocatedBuffer* out) { EVPKeyPointer pkey; - auto* allocator = env->isolate()->GetArrayBufferAllocator(); // Check if this is a PKCS#8 or RSA public key before trying as X.509 and // private key. @@ -3879,14 +3890,21 @@ bool PublicKeyCipher::Cipher(Environment* env, if (EVP_PKEY_CTX_set_rsa_padding(ctx.get(), padding) <= 0) return false; - if (EVP_PKEY_cipher(ctx.get(), nullptr, out_len, data, len) <= 0) + size_t out_len = 0; + if (EVP_PKEY_cipher(ctx.get(), nullptr, &out_len, data, len) <= 0) return false; - *out = static_cast(allocator->AllocateUninitialized(*out_len)); + *out = env->AllocateManaged(out_len); - if (EVP_PKEY_cipher(ctx.get(), *out, out_len, data, len) <= 0) + if (EVP_PKEY_cipher(ctx.get(), + reinterpret_cast(out->data()), + &out_len, + data, + len) <= 0) { return false; + } + out->Resize(out_len); return true; } @@ -3910,8 +3928,7 @@ void PublicKeyCipher::Cipher(const FunctionCallbackInfo& args) { String::Utf8Value passphrase(args.GetIsolate(), args[3]); - unsigned char* out_value = nullptr; - size_t out_len = 0; + AllocatedBuffer out; ClearErrorOnReturn clear_error_on_return; @@ -3923,24 +3940,12 @@ void PublicKeyCipher::Cipher(const FunctionCallbackInfo& args) { padding, reinterpret_cast(buf), len, - &out_value, - &out_len); - - if (out_len == 0 || !r) { - auto* allocator = env->isolate()->GetArrayBufferAllocator(); - allocator->Free(out_value, out_len); - out_value = nullptr; - out_len = 0; - if (!r) { - return ThrowCryptoError(env, - ERR_get_error()); - } - } + &out); - Local vbuf = - Buffer::New(env, reinterpret_cast(out_value), out_len) - .ToLocalChecked(); - args.GetReturnValue().Set(vbuf); + if (!r) + return ThrowCryptoError(env, ERR_get_error()); + + args.GetReturnValue().Set(out.ToBuffer().ToLocalChecked()); } @@ -4141,10 +4146,9 @@ void DiffieHellman::GenerateKeys(const FunctionCallbackInfo& args) { const BIGNUM* pub_key; DH_get0_key(diffieHellman->dh_.get(), &pub_key, nullptr); size_t size = BN_num_bytes(pub_key); - auto* allocator = env->isolate()->GetArrayBufferAllocator(); - char* data = static_cast(allocator->AllocateUninitialized(size)); - BN_bn2bin(pub_key, reinterpret_cast(data)); - args.GetReturnValue().Set(Buffer::New(env, data, size).ToLocalChecked()); + AllocatedBuffer data = env->AllocateManaged(size); + BN_bn2bin(pub_key, reinterpret_cast(data.data())); + args.GetReturnValue().Set(data.ToBuffer().ToLocalChecked()); } @@ -4161,10 +4165,9 @@ void DiffieHellman::GetField(const FunctionCallbackInfo& args, if (num == nullptr) return env->ThrowError(err_if_null); size_t size = BN_num_bytes(num); - auto* allocator = env->isolate()->GetArrayBufferAllocator(); - char* data = static_cast(allocator->AllocateUninitialized(size)); - BN_bn2bin(num, reinterpret_cast(data)); - args.GetReturnValue().Set(Buffer::New(env, data, size).ToLocalChecked()); + AllocatedBuffer data = env->AllocateManaged(size); + BN_bn2bin(num, reinterpret_cast(data.data())); + args.GetReturnValue().Set(data.ToBuffer().ToLocalChecked()); } void DiffieHellman::GetPrime(const FunctionCallbackInfo& args) { @@ -4226,10 +4229,9 @@ void DiffieHellman::ComputeSecret(const FunctionCallbackInfo& args) { Buffer::Length(args[0]), 0)); - auto* allocator = env->isolate()->GetArrayBufferAllocator(); - MallocedBuffer data(DH_size(diffieHellman->dh_.get()), allocator); + AllocatedBuffer ret = env->AllocateManaged(DH_size(diffieHellman->dh_.get())); - int size = DH_compute_key(reinterpret_cast(data.data), + int size = DH_compute_key(reinterpret_cast(ret.data()), key.get(), diffieHellman->dh_.get()); @@ -4264,14 +4266,13 @@ void DiffieHellman::ComputeSecret(const FunctionCallbackInfo& args) { // DH_compute_key returns number of bytes in a remainder of exponent, which // may have less bytes than a prime number. Therefore add 0-padding to the // allocated buffer. - if (static_cast(size) != data.size) { - CHECK_GT(data.size, static_cast(size)); - memmove(data.data + data.size - size, data.data, size); - memset(data.data, 0, data.size - size); + if (static_cast(size) != ret.size()) { + CHECK_GT(ret.size(), static_cast(size)); + memmove(ret.data() + ret.size() - size, ret.data(), size); + memset(ret.data(), 0, ret.size() - size); } - args.GetReturnValue().Set( - Buffer::New(env->isolate(), data.release(), data.size).ToLocalChecked()); + args.GetReturnValue().Set(ret.ToBuffer().ToLocalChecked()); } void DiffieHellman::SetKey(const v8::FunctionCallbackInfo& args, @@ -4447,18 +4448,16 @@ void ECDH::ComputeSecret(const FunctionCallbackInfo& args) { } // NOTE: field_size is in bits - auto* allocator = env->isolate()->GetArrayBufferAllocator(); int field_size = EC_GROUP_get_degree(ecdh->group_); size_t out_len = (field_size + 7) / 8; - char* out = static_cast(allocator->AllocateUninitialized(out_len)); + AllocatedBuffer out = env->AllocateManaged(out_len); - int r = ECDH_compute_key(out, out_len, pub.get(), ecdh->key_.get(), nullptr); - if (!r) { - allocator->Free(out, out_len); + int r = ECDH_compute_key( + out.data(), out_len, pub.get(), ecdh->key_.get(), nullptr); + if (!r) return env->ThrowError("Failed to compute ECDH key"); - } - Local buf = Buffer::New(env, out, out_len).ToLocalChecked(); + Local buf = out.ToBuffer().ToLocalChecked(); args.GetReturnValue().Set(buf); } @@ -4472,31 +4471,19 @@ void ECDH::GetPublicKey(const FunctionCallbackInfo& args) { ECDH* ecdh; ASSIGN_OR_RETURN_UNWRAP(&ecdh, args.Holder()); + const EC_GROUP* group = EC_KEY_get0_group(ecdh->key_.get()); const EC_POINT* pub = EC_KEY_get0_public_key(ecdh->key_.get()); if (pub == nullptr) return env->ThrowError("Failed to get ECDH public key"); - int size; CHECK(args[0]->IsUint32()); uint32_t val = args[0].As()->Value(); point_conversion_form_t form = static_cast(val); - size = EC_POINT_point2oct(ecdh->group_, pub, form, nullptr, 0, nullptr); - if (size == 0) - return env->ThrowError("Failed to get public key length"); - - auto* allocator = env->isolate()->GetArrayBufferAllocator(); - unsigned char* out = - static_cast(allocator->AllocateUninitialized(size)); - - int r = EC_POINT_point2oct(ecdh->group_, pub, form, out, size, nullptr); - if (r != size) { - allocator->Free(out, size); - return env->ThrowError("Failed to get public key"); - } - - Local buf = - Buffer::New(env, reinterpret_cast(out), size).ToLocalChecked(); + const char* error; + Local buf; + if (!ECPointToBuffer(env, group, pub, form, &error).ToLocal(&buf)) + return env->ThrowError(error); args.GetReturnValue().Set(buf); } @@ -4511,18 +4498,14 @@ void ECDH::GetPrivateKey(const FunctionCallbackInfo& args) { if (b == nullptr) return env->ThrowError("Failed to get ECDH private key"); - auto* allocator = env->isolate()->GetArrayBufferAllocator(); int size = BN_num_bytes(b); - unsigned char* out = - static_cast(allocator->AllocateUninitialized(size)); + AllocatedBuffer out = env->AllocateManaged(size); - if (size != BN_bn2bin(b, out)) { - allocator->Free(out, size); + if (size != BN_bn2bin(b, reinterpret_cast(out.data()))) { return env->ThrowError("Failed to convert ECDH private key to Buffer"); } - Local buf = - Buffer::New(env, reinterpret_cast(out), size).ToLocalChecked(); + Local buf = out.ToBuffer().ToLocalChecked(); args.GetReturnValue().Set(buf); } @@ -4985,32 +4968,28 @@ void VerifySpkac(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(verify_result); } - -char* ExportPublicKey(Environment* env, const char* data, int len, size_t* size) { - char* buf = nullptr; - auto* allocator = env->isolate()->GetArrayBufferAllocator(); - +AllocatedBuffer ExportPublicKey(Environment* env, + const char* data, + int len, + size_t* size) { BIOPointer bio(BIO_new(BIO_s_mem())); - if (!bio) - return nullptr; + if (!bio) return AllocatedBuffer(); NetscapeSPKIPointer spki(NETSCAPE_SPKI_b64_decode(data, len)); - if (!spki) - return nullptr; + if (!spki) return AllocatedBuffer(); EVPKeyPointer pkey(NETSCAPE_SPKI_get_pubkey(spki.get())); - if (!pkey) - return nullptr; + if (!pkey) return AllocatedBuffer(); if (PEM_write_bio_PUBKEY(bio.get(), pkey.get()) <= 0) - return nullptr; + return AllocatedBuffer(); BUF_MEM* ptr; BIO_get_mem_ptr(bio.get(), &ptr); *size = ptr->length; - buf = static_cast(allocator->AllocateUninitialized(*size)); - memcpy(buf, ptr->data, *size); + AllocatedBuffer buf = env->AllocateManaged(*size); + memcpy(buf.data(), ptr->data, *size); return buf; } @@ -5027,12 +5006,11 @@ void ExportPublicKey(const FunctionCallbackInfo& args) { CHECK_NOT_NULL(data); size_t pkey_size; - char* pkey = ExportPublicKey(env, data, length, &pkey_size); - if (pkey == nullptr) + AllocatedBuffer pkey = ExportPublicKey(env, data, length, &pkey_size); + if (pkey.data() == nullptr) return args.GetReturnValue().SetEmptyString(); - Local out = Buffer::New(env, pkey, pkey_size).ToLocalChecked(); - args.GetReturnValue().Set(out); + args.GetReturnValue().Set(pkey.ToBuffer().ToLocalChecked()); } @@ -5103,24 +5081,10 @@ void ConvertKey(const FunctionCallbackInfo& args) { uint32_t val = args[2].As()->Value(); point_conversion_form_t form = static_cast(val); - int size = EC_POINT_point2oct( - group.get(), pub.get(), form, nullptr, 0, nullptr); - - if (size == 0) - return env->ThrowError("Failed to get public key length"); - - auto* allocator = env->isolate()->GetArrayBufferAllocator(); - unsigned char* out = - static_cast(allocator->AllocateUninitialized(size)); - - int r = EC_POINT_point2oct(group.get(), pub.get(), form, out, size, nullptr); - if (r != size) { - allocator->Free(out, size); - return env->ThrowError("Failed to get public key"); - } - - Local buf = - Buffer::New(env, reinterpret_cast(out), size).ToLocalChecked(); + const char* error; + Local buf; + if (!ECPointToBuffer(env, group.get(), pub.get(), form, &error).ToLocal(&buf)) + return env->ThrowError(error); args.GetReturnValue().Set(buf); } diff --git a/src/node_crypto.h b/src/node_crypto.h index 2da3bc7cac5..b9bff4adde4 100644 --- a/src/node_crypto.h +++ b/src/node_crypto.h @@ -383,9 +383,8 @@ class CipherBase : public BaseObject { bool InitAuthenticated(const char* cipher_type, int iv_len, unsigned int auth_tag_len); bool CheckCCMMessageLength(int message_len); - UpdateResult Update(const char* data, int len, unsigned char** out, - int* out_len); - bool Final(unsigned char** out, int* out_len); + UpdateResult Update(const char* data, int len, AllocatedBuffer* out); + bool Final(AllocatedBuffer* out); bool SetAutoPadding(bool auto_padding); bool IsAuthenticatedMode() const; @@ -583,8 +582,7 @@ class PublicKeyCipher { int padding, const unsigned char* data, int len, - unsigned char** out, - size_t* out_len); + AllocatedBuffer* out); template ` containing the TypeError with proper code and message #define ERRORS_WITH_CODE(V) \ + V(ERR_BUFFER_CONTEXT_NOT_AVAILABLE, Error) \ V(ERR_BUFFER_OUT_OF_BOUNDS, RangeError) \ V(ERR_BUFFER_TOO_LARGE, Error) \ V(ERR_CANNOT_TRANSFER_OBJECT, TypeError) \ @@ -55,6 +56,8 @@ namespace node { // Errors with predefined static messages #define PREDEFINED_ERROR_MESSAGES(V) \ + V(ERR_BUFFER_CONTEXT_NOT_AVAILABLE, \ + "Buffer is not available for the current Context") \ V(ERR_CANNOT_TRANSFER_OBJECT, "Cannot transfer object of unsupported type")\ V(ERR_CLOSED_MESSAGE_PORT, "Cannot send data on closed MessagePort") \ V(ERR_CONSTRUCT_CALL_REQUIRED, "Cannot call constructor without `new`") \ @@ -73,8 +76,11 @@ namespace node { inline v8::Local code(v8::Isolate* isolate) { \ return code(isolate, message); \ } \ + inline void THROW_ ## code(v8::Isolate* isolate) { \ + isolate->ThrowException(code(isolate, message)); \ + } \ inline void THROW_ ## code(Environment* env) { \ - env->isolate()->ThrowException(code(env->isolate(), message)); \ + THROW_ ## code(env->isolate()); \ } PREDEFINED_ERROR_MESSAGES(V) #undef V diff --git a/src/node_http2.cc b/src/node_http2.cc index 8841601040c..dcfa1694f93 100644 --- a/src/node_http2.cc +++ b/src/node_http2.cc @@ -1678,47 +1678,45 @@ Http2Stream* Http2Session::SubmitRequest( return stream; } +uv_buf_t Http2Session::OnStreamAlloc(size_t suggested_size) { + return env()->AllocateManaged(suggested_size).release(); +} + // Callback used to receive inbound data from the i/o stream -void Http2Session::OnStreamRead(ssize_t nread, const uv_buf_t& buf) { +void Http2Session::OnStreamRead(ssize_t nread, const uv_buf_t& buf_) { HandleScope handle_scope(env()->isolate()); Context::Scope context_scope(env()->context()); Http2Scope h2scope(this); CHECK_NOT_NULL(stream_); Debug(this, "receiving %d bytes", nread); - IncrementCurrentSessionMemory(buf.len); CHECK(stream_buf_ab_.IsEmpty()); + AllocatedBuffer buf(env(), buf_); if (nread <= 0) { - free(buf.base); if (nread < 0) { PassReadErrorToPreviousListener(nread); } } else { // Only pass data on if nread > 0 + // Shrink to the actual amount of used data. + buf.Resize(nread); + + IncrementCurrentSessionMemory(nread); + // Makre sure that there was no read previously active. CHECK_NULL(stream_buf_.base); CHECK_EQ(stream_buf_.len, 0); // Remember the current buffer, so that OnDataChunkReceived knows the // offset of a DATA frame's data into the socket read buffer. - stream_buf_ = uv_buf_init(buf.base, nread); - - // Verify that currently: There is memory allocated into which - // the data has been read, and that memory buffer is at least as large - // as the amount of data we have read, but we have not yet made an - // ArrayBuffer out of it. - CHECK_LE(static_cast(nread), stream_buf_.len); + stream_buf_ = uv_buf_init(buf.data(), nread); Isolate* isolate = env()->isolate(); // Create an array buffer for the read data. DATA frames will be emitted // as slices of this array buffer to avoid having to copy memory. - stream_buf_ab_ = - ArrayBuffer::New(isolate, - buf.base, - nread, - v8::ArrayBufferCreationMode::kInternalized); + stream_buf_ab_ = buf.ToArrayBuffer(); statistics_.data_received += nread; ssize_t ret = Write(&stream_buf_, 1); @@ -1737,7 +1735,7 @@ void Http2Session::OnStreamRead(ssize_t nread, const uv_buf_t& buf) { // Since we are finished handling this write, reset the stream buffer. // The memory has either been free()d or was handed over to V8. - DecrementCurrentSessionMemory(buf.len); + DecrementCurrentSessionMemory(nread); stream_buf_ab_ = Local(); stream_buf_ = uv_buf_init(nullptr, 0); diff --git a/src/node_http2.h b/src/node_http2.h index d7f8d9acae9..8ea6e5fc36f 100644 --- a/src/node_http2.h +++ b/src/node_http2.h @@ -779,6 +779,7 @@ class Http2Session : public AsyncWrap, public StreamListener { } // Handle reads/writes from the underlying network transport. + uv_buf_t OnStreamAlloc(size_t suggested_size) override; void OnStreamRead(ssize_t nread, const uv_buf_t& buf) override; void OnStreamAfterWrite(WriteWrap* w, int status) override; diff --git a/src/node_http_parser.cc b/src/node_http_parser.cc index a4b92cbb20b..852f925b9bc 100644 --- a/src/node_http_parser.cc +++ b/src/node_http_parser.cc @@ -531,10 +531,9 @@ class Parser : public AsyncWrap, public StreamListener { uv_buf_t OnStreamAlloc(size_t suggested_size) override { // For most types of streams, OnStreamRead will be immediately after // OnStreamAlloc, and will consume all data, so using a static buffer for - // reading is more efficient. For other streams, just use the default - // allocator, which uses Malloc(). + // reading is more efficient. For other streams, just use Malloc() directly. if (env()->http_parser_buffer_in_use()) - return StreamListener::OnStreamAlloc(suggested_size); + return uv_buf_init(Malloc(suggested_size), suggested_size); env()->set_http_parser_buffer_in_use(true); if (env()->http_parser_buffer() == nullptr) diff --git a/src/node_internals.h b/src/node_internals.h index fe446dbc713..9f4fdf2e162 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -398,15 +398,38 @@ class ArrayBufferAllocator : public v8::ArrayBuffer::Allocator { public: inline uint32_t* zero_fill_field() { return &zero_fill_field_; } - virtual void* Allocate(size_t size); // Defined in src/node.cc - virtual void* AllocateUninitialized(size_t size) + void* Allocate(size_t size) override; // Defined in src/node.cc + void* AllocateUninitialized(size_t size) override { return node::UncheckedMalloc(size); } - virtual void Free(void* data, size_t) { free(data); } + void Free(void* data, size_t) override { free(data); } + virtual void* Reallocate(void* data, size_t old_size, size_t size) { + return static_cast( + UncheckedRealloc(static_cast(data), size)); + } + virtual void RegisterPointer(void* data, size_t size) {} + virtual void UnregisterPointer(void* data, size_t size) {} private: uint32_t zero_fill_field_ = 1; // Boolean but exposed as uint32 to JS land. }; +class DebuggingArrayBufferAllocator final : public ArrayBufferAllocator { + public: + ~DebuggingArrayBufferAllocator() override; + void* Allocate(size_t size) override; + void* AllocateUninitialized(size_t size) override; + void Free(void* data, size_t size) override; + void* Reallocate(void* data, size_t old_size, size_t size) override; + void RegisterPointer(void* data, size_t size) override; + void UnregisterPointer(void* data, size_t size) override; + + private: + void RegisterPointerInternal(void* data, size_t size); + void UnregisterPointerInternal(void* data, size_t size); + Mutex mutex_; + std::unordered_map allocations_; +}; + namespace Buffer { v8::MaybeLocal Copy(Environment* env, const char* data, size_t len); v8::MaybeLocal New(Environment* env, size_t size); @@ -416,10 +439,12 @@ v8::MaybeLocal New(Environment* env, size_t length, void (*callback)(char* data, void* hint), void* hint); -// Takes ownership of |data|. Must allocate |data| with malloc() or realloc() -// because ArrayBufferAllocator::Free() deallocates it again with free(). -// Mixing operator new and free() is undefined behavior so don't do that. -v8::MaybeLocal New(Environment* env, char* data, size_t length); +// Takes ownership of |data|. Must allocate |data| with the current Isolate's +// ArrayBuffer::Allocator(). +v8::MaybeLocal New(Environment* env, + char* data, + size_t length, + bool uses_malloc); inline v8::MaybeLocal New(Environment* env, @@ -450,7 +475,7 @@ static v8::MaybeLocal New(Environment* env, const size_t len_in_bytes = buf->length() * sizeof(buf->out()[0]); if (buf->IsAllocated()) - ret = New(env, src, len_in_bytes); + ret = New(env, src, len_in_bytes, true); else if (!buf->IsInvalidated()) ret = Copy(env, src, len_in_bytes); diff --git a/src/node_messaging.cc b/src/node_messaging.cc index 2271ed3f74b..7d6c6ad80cd 100644 --- a/src/node_messaging.cc +++ b/src/node_messaging.cc @@ -118,6 +118,21 @@ MaybeLocal Message::Deserialize(Environment* env, // Attach all transfered ArrayBuffers to their new Isolate. for (uint32_t i = 0; i < array_buffer_contents_.size(); ++i) { + if (!env->isolate_data()->uses_node_allocator()) { + // We don't use Node's allocator on the receiving side, so we have + // to create the ArrayBuffer from a copy of the memory. + AllocatedBuffer buf = + env->AllocateManaged(array_buffer_contents_[i].size); + memcpy(buf.data(), + array_buffer_contents_[i].data, + array_buffer_contents_[i].size); + deserializer.TransferArrayBuffer(i, buf.ToArrayBuffer()); + continue; + } + + env->isolate_data()->node_allocator()->RegisterPointer( + array_buffer_contents_[i].data, array_buffer_contents_[i].size); + Local ab = ArrayBuffer::New(env->isolate(), array_buffer_contents_[i].release(), @@ -164,12 +179,8 @@ void ThrowDataCloneException(Environment* env, Local message) { // DeserializerDelegate understands how to unpack. class SerializerDelegate : public ValueSerializer::Delegate { public: - SerializerDelegate( - Environment* env, - Local context, - Message* m, - v8::ArrayBuffer::Allocator *allocator) - : env_(env), context_(context), msg_(m), allocator_(allocator) {} + SerializerDelegate(Environment* env, Local context, Message* m) + : env_(env), context_(context), msg_(m) {} void ThrowDataCloneError(Local message) override { ThrowDataCloneException(env_, message); @@ -205,19 +216,6 @@ class SerializerDelegate : public ValueSerializer::Delegate { return Just(i); } - void FreeBufferMemory(void* buffer) override { - // the second parameter is unused, so pass 0 as a filler - return allocator_->Free(buffer, 0); - } - - void* ReallocateBufferMemory(void* old_buffer, - size_t size, - size_t* actual_size) override { - auto result = allocator_->Realloc(old_buffer, size); - *actual_size = result ? size :0; - return result; - } - void Finish() { // Only close the MessagePort handles and actually transfer them // once we know that serialization succeeded. @@ -247,7 +245,6 @@ class SerializerDelegate : public ValueSerializer::Delegate { Message* msg_; std::vector> seen_shared_array_buffers_; std::vector ports_; - v8::ArrayBuffer::Allocator* allocator_; friend class worker::Message; }; @@ -265,7 +262,7 @@ Maybe Message::Serialize(Environment* env, // Verify that we're not silently overwriting an existing message. CHECK(main_message_buf_.is_empty()); - SerializerDelegate delegate(env, context, this, env->isolate()->GetArrayBufferAllocator()); + SerializerDelegate delegate(env, context, this); ValueSerializer serializer(env->isolate(), &delegate); delegate.serializer = &serializer; @@ -282,8 +279,10 @@ Maybe Message::Serialize(Environment* env, Local ab = entry.As(); // If we cannot render the ArrayBuffer unusable in this Isolate and // take ownership of its memory, copying the buffer will have to do. - if (!ab->IsNeuterable() || ab->IsExternal()) + if (!ab->IsNeuterable() || ab->IsExternal() || + !env->isolate_data()->uses_node_allocator()) { continue; + } // We simply use the array index in the `array_buffers` list as the // ID that we write into the serialized buffer. uint32_t id = array_buffers.size(); @@ -329,10 +328,14 @@ Maybe Message::Serialize(Environment* env, // it inaccessible in this Isolate. ArrayBuffer::Contents contents = ab->Externalize(); ab->Neuter(); + + CHECK(env->isolate_data()->uses_node_allocator()); + env->isolate_data()->node_allocator()->UnregisterPointer( + contents.Data(), contents.ByteLength()); + array_buffer_contents_.push_back( MallocedBuffer { static_cast(contents.Data()), - contents.ByteLength(), - env->isolate()->GetArrayBufferAllocator() }); + contents.ByteLength() }); } delegate.Finish(); @@ -340,7 +343,7 @@ Maybe Message::Serialize(Environment* env, // The serializer gave us a buffer allocated using `malloc()`. std::pair data = serializer.Release(); main_message_buf_ = - MallocedBuffer(reinterpret_cast(data.first), data.second, env->isolate()->GetArrayBufferAllocator()); + MallocedBuffer(reinterpret_cast(data.first), data.second); return Just(true); } diff --git a/src/node_options.cc b/src/node_options.cc index 5cce1179b79..c34e29fdda3 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -223,6 +223,10 @@ PerProcessOptionsParser::PerProcessOptionsParser() { "SlowBuffer instances", &PerProcessOptions::zero_fill_all_buffers, kAllowedInEnvironment); + AddOption("--debug-arraybuffer-allocations", + "", /* undocumented, only for debugging */ + &PerProcessOptions::debug_arraybuffer_allocations, + kAllowedInEnvironment); AddOption("--security-reverts", "", &PerProcessOptions::security_reverts); AddOption("--help", diff --git a/src/node_options.h b/src/node_options.h index ec0179d890f..c19099dfa2d 100644 --- a/src/node_options.h +++ b/src/node_options.h @@ -111,6 +111,7 @@ class PerProcessOptions { std::string trace_event_file_pattern = "node_trace.${rotation}.log"; int64_t v8_thread_pool_size = 4; bool zero_fill_all_buffers = false; + bool debug_arraybuffer_allocations = false; std::vector security_reverts; bool print_help = false; diff --git a/src/node_serdes.cc b/src/node_serdes.cc index 5de0ddd8190..79df651664e 100644 --- a/src/node_serdes.cc +++ b/src/node_serdes.cc @@ -205,10 +205,13 @@ void SerializerContext::ReleaseBuffer(const FunctionCallbackInfo& args) { SerializerContext* ctx; ASSIGN_OR_RETURN_UNWRAP(&ctx, args.Holder()); + // Note: Both ValueSerializer and this Buffer::New() variant use malloc() + // as the underlying allocator. std::pair ret = ctx->serializer_.Release(); auto buf = Buffer::New(ctx->env(), reinterpret_cast(ret.first), - ret.second); + ret.second, + true /* uses_malloc */); if (!buf.IsEmpty()) { args.GetReturnValue().Set(buf.ToLocalChecked()); diff --git a/src/stream_base-inl.h b/src/stream_base-inl.h index 027b938d30d..21cf0d6d94b 100644 --- a/src/stream_base-inl.h +++ b/src/stream_base-inl.h @@ -412,18 +412,13 @@ inline void ShutdownWrap::OnDone(int status) { Dispose(); } -inline void WriteWrap::SetAllocatedStorage(char* data, size_t size) { - CHECK_NULL(storage_); - storage_ = data; - storage_size_ = size; -} - -inline char* WriteWrap::Storage() { - return storage_; +inline size_t WriteWrap::StorageSize() const { + return storage_.size(); } -inline size_t WriteWrap::StorageSize() const { - return storage_size_; +inline void WriteWrap::SetAllocatedStorage(AllocatedBuffer&& storage) { + CHECK_NULL(storage_.data()); + storage_ = std::move(storage); } inline void WriteWrap::OnDone(int status) { diff --git a/src/stream_base.cc b/src/stream_base.cc index dcba9e1ffa9..c8400f69833 100644 --- a/src/stream_base.cc +++ b/src/stream_base.cc @@ -127,9 +127,9 @@ int StreamBase::Writev(const FunctionCallbackInfo& args) { } } - std::unique_ptr storage; + AllocatedBuffer storage; if (storage_size > 0) - storage = std::unique_ptr(Malloc(storage_size)); + storage = env->AllocateManaged(storage_size); offset = 0; if (!all_buffers) { @@ -145,8 +145,8 @@ int StreamBase::Writev(const FunctionCallbackInfo& args) { // Write string CHECK_LE(offset, storage_size); - char* str_storage = storage.get() + offset; - size_t str_size = storage_size - offset; + char* str_storage = storage.data() + offset; + size_t str_size = storage.size() - offset; Local string = chunk->ToString(env->context()).ToLocalChecked(); enum encoding encoding = ParseEncoding(env->isolate(), @@ -164,8 +164,8 @@ int StreamBase::Writev(const FunctionCallbackInfo& args) { StreamWriteResult res = Write(*bufs, count, nullptr, req_wrap_obj); SetWriteResultPropertiesOnWrapObject(env, req_wrap_obj, res); - if (res.wrap != nullptr && storage) { - res.wrap->SetAllocatedStorage(storage.release(), storage_size); + if (res.wrap != nullptr && storage_size > 0) { + res.wrap->SetAllocatedStorage(std::move(storage)); } return res.err; } @@ -265,18 +265,18 @@ int StreamBase::WriteString(const FunctionCallbackInfo& args) { CHECK_EQ(count, 1); } - std::unique_ptr data; + AllocatedBuffer data; if (try_write) { // Copy partial data - data = std::unique_ptr(Malloc(buf.len)); - memcpy(data.get(), buf.base, buf.len); + data = env->AllocateManaged(buf.len); + memcpy(data.data(), buf.base, buf.len); data_size = buf.len; } else { // Write it - data = std::unique_ptr(Malloc(storage_size)); + data = env->AllocateManaged(storage_size); data_size = StringBytes::Write(env->isolate(), - data.get(), + data.data(), storage_size, string, enc); @@ -284,7 +284,7 @@ int StreamBase::WriteString(const FunctionCallbackInfo& args) { CHECK_LE(data_size, storage_size); - buf = uv_buf_init(data.get(), data_size); + buf = uv_buf_init(data.data(), data_size); uv_stream_t* send_handle = nullptr; @@ -302,7 +302,7 @@ int StreamBase::WriteString(const FunctionCallbackInfo& args) { SetWriteResultPropertiesOnWrapObject(env, req_wrap_obj, res); if (res.wrap != nullptr) { - res.wrap->SetAllocatedStorage(data.release(), data_size); + res.wrap->SetAllocatedStorage(std::move(data)); } return res.err; @@ -356,38 +356,30 @@ void StreamResource::ClearError() { // No-op } - -uv_buf_t StreamListener::OnStreamAlloc(size_t suggested_size) { - CHECK_NE(stream_, nullptr); - StreamBase* stream = static_cast(stream_); - Environment* env = stream->stream_env(); - auto* allocator = env->isolate()->GetArrayBufferAllocator(); - return uv_buf_init( - static_cast(allocator->AllocateUninitialized(suggested_size)), - suggested_size); +uv_buf_t EmitToJSStreamListener::OnStreamAlloc(size_t suggested_size) { + CHECK_NOT_NULL(stream_); + Environment* env = static_cast(stream_)->stream_env(); + return env->AllocateManaged(suggested_size).release(); } - -void EmitToJSStreamListener::OnStreamRead(ssize_t nread, const uv_buf_t& buf) { +void EmitToJSStreamListener::OnStreamRead(ssize_t nread, const uv_buf_t& buf_) { CHECK_NOT_NULL(stream_); StreamBase* stream = static_cast(stream_); Environment* env = stream->stream_env(); - auto* allocator = env->isolate()->GetArrayBufferAllocator(); HandleScope handle_scope(env->isolate()); Context::Scope context_scope(env->context()); + AllocatedBuffer buf(env, buf_); if (nread <= 0) { - allocator->Free(buf.base, buf.len); if (nread < 0) stream->CallJSOnreadMethod(nread, Local()); return; } - CHECK_LE(static_cast(nread), buf.len); - char* base = static_cast(allocator->Realloc(buf.base, nread)); + CHECK_LE(static_cast(nread), buf.size()); + buf.Resize(nread); - Local obj = Buffer::New(env, base, nread).ToLocalChecked(); - stream->CallJSOnreadMethod(nread, obj); + stream->CallJSOnreadMethod(nread, buf.ToBuffer().ToLocalChecked()); } diff --git a/src/stream_base.h b/src/stream_base.h index 05c2a962362..597ae0c9062 100644 --- a/src/stream_base.h +++ b/src/stream_base.h @@ -74,24 +74,18 @@ class ShutdownWrap : public StreamReq { class WriteWrap : public StreamReq { public: - char* Storage(); size_t StorageSize() const; - void SetAllocatedStorage(char* data, size_t size); + void SetAllocatedStorage(AllocatedBuffer&& storage); WriteWrap(StreamBase* stream, v8::Local req_wrap_obj) : StreamReq(stream, req_wrap_obj) { } - ~WriteWrap() { - free(storage_); - } - // Call stream()->EmitAfterWrite() and dispose of this request wrap. void OnDone(int status) override; private: - char* storage_ = nullptr; - size_t storage_size_ = 0; + AllocatedBuffer storage_; }; @@ -115,7 +109,7 @@ class StreamListener { // It is not valid to return a zero-length buffer from this method. // It is not guaranteed that the corresponding `OnStreamRead()` call // happens in the same event loop turn as this call. - virtual uv_buf_t OnStreamAlloc(size_t suggested_size); + virtual uv_buf_t OnStreamAlloc(size_t suggested_size) = 0; // `OnStreamRead()` is called when data is available on the socket and has // been read into the buffer provided by `OnStreamAlloc()`. @@ -181,6 +175,7 @@ class ReportWritesToJSStreamListener : public StreamListener { // JS land via the handle’s .ondata method. class EmitToJSStreamListener : public ReportWritesToJSStreamListener { public: + uv_buf_t OnStreamAlloc(size_t suggested_size) override; void OnStreamRead(ssize_t nread, const uv_buf_t& buf) override; }; diff --git a/src/stream_pipe.cc b/src/stream_pipe.cc index e19f98e35d2..2444b6edb8b 100644 --- a/src/stream_pipe.cc +++ b/src/stream_pipe.cc @@ -109,17 +109,17 @@ uv_buf_t StreamPipe::ReadableListener::OnStreamAlloc(size_t suggested_size) { StreamPipe* pipe = ContainerOf(&StreamPipe::readable_listener_, this); size_t size = std::min(suggested_size, pipe->wanted_data_); CHECK_GT(size, 0); - return uv_buf_init(Malloc(size), size); + return pipe->env()->AllocateManaged(size).release(); } void StreamPipe::ReadableListener::OnStreamRead(ssize_t nread, - const uv_buf_t& buf) { + const uv_buf_t& buf_) { StreamPipe* pipe = ContainerOf(&StreamPipe::readable_listener_, this); + AllocatedBuffer buf(pipe->env(), buf_); AsyncScope async_scope(pipe); if (nread < 0) { // EOF or error; stop reading and pass the error to the previous listener // (which might end up in JS). - free(buf.base); pipe->is_eof_ = true; stream()->ReadStop(); CHECK_NOT_NULL(previous_listener_); @@ -133,19 +133,18 @@ void StreamPipe::ReadableListener::OnStreamRead(ssize_t nread, return; } - pipe->ProcessData(nread, buf); + pipe->ProcessData(nread, std::move(buf)); } -void StreamPipe::ProcessData(size_t nread, const uv_buf_t& buf) { - uv_buf_t buffer = uv_buf_init(buf.base, nread); +void StreamPipe::ProcessData(size_t nread, AllocatedBuffer&& buf) { + uv_buf_t buffer = uv_buf_init(buf.data(), nread); StreamWriteResult res = sink()->Write(&buffer, 1); if (!res.async) { - free(buf.base); writable_listener_.OnStreamAfterWrite(nullptr, res.err); } else { is_writing_ = true; is_reading_ = false; - res.wrap->SetAllocatedStorage(buf.base, buf.len); + res.wrap->SetAllocatedStorage(std::move(buf)); if (source() != nullptr) source()->ReadStop(); } diff --git a/src/stream_pipe.h b/src/stream_pipe.h index c76afac4168..ea9ea853fed 100644 --- a/src/stream_pipe.h +++ b/src/stream_pipe.h @@ -44,7 +44,7 @@ class StreamPipe : public AsyncWrap { // `OnStreamWantsWrite()` support. size_t wanted_data_ = 0; - void ProcessData(size_t nread, const uv_buf_t& buf); + void ProcessData(size_t nread, AllocatedBuffer&& buf); class ReadableListener : public StreamListener { public: diff --git a/src/udp_wrap.cc b/src/udp_wrap.cc index 9ed9a5a71f4..b201a40e4c8 100644 --- a/src/udp_wrap.cc +++ b/src/udp_wrap.cc @@ -455,26 +455,20 @@ void UDPWrap::OnSend(uv_udp_send_t* req, int status) { void UDPWrap::OnAlloc(uv_handle_t* handle, size_t suggested_size, uv_buf_t* buf) { - auto* wrap = static_cast(handle->data); - auto* allocator = wrap->env()->isolate()->GetArrayBufferAllocator(); - buf->base = - static_cast(allocator->AllocateUninitialized(suggested_size)); - buf->len = suggested_size; + UDPWrap* wrap = static_cast(handle->data); + *buf = wrap->env()->AllocateManaged(suggested_size).release(); } - void UDPWrap::OnRecv(uv_udp_t* handle, ssize_t nread, - const uv_buf_t* buf, + const uv_buf_t* buf_, const struct sockaddr* addr, unsigned int flags) { UDPWrap* wrap = static_cast(handle->data); Environment* env = wrap->env(); - auto* allocator = env->isolate()->GetArrayBufferAllocator(); + AllocatedBuffer buf(env, *buf_); if (nread == 0 && addr == nullptr) { - if (buf->base != nullptr) - allocator->Free(buf->base, buf->len); return; } @@ -490,16 +484,12 @@ void UDPWrap::OnRecv(uv_udp_t* handle, }; if (nread < 0) { - if (buf->base != nullptr) - allocator->Free(buf->base, buf->len); wrap->MakeCallback(env->onmessage_string(), arraysize(argv), argv); return; } - // Note that nread may be smaller thant buf->len, in that case the length - // passed to ArrayBufferAllocator::Free would not be the correct one, but - // it should be fine unless embedder is using some unusual memory allocator. - argv[2] = Buffer::New(env, buf->base, nread).ToLocalChecked(); + buf.Resize(nread); + argv[2] = buf.ToBuffer().ToLocalChecked(); argv[3] = AddressToJS(env, addr); wrap->MakeCallback(env->onmessage_string(), arraysize(argv), argv); } diff --git a/src/util.h b/src/util.h index 8453762a728..4036383dee9 100644 --- a/src/util.h +++ b/src/util.h @@ -1,4 +1,5 @@ // Copyright Joyent, Inc. and other Node contributors. +// // Permission is hereby granted, free of charge, to any person obtaining a // copy of this software and associated documentation files (the // "Software"), to deal in the Software without restriction, including @@ -439,10 +440,8 @@ template struct MallocedBuffer { T* data; size_t size; - v8::ArrayBuffer::Allocator* allocator; T* release() { - allocator = nullptr; T* ret = data; data = nullptr; return ret; @@ -450,28 +449,18 @@ struct MallocedBuffer { inline bool is_empty() const { return data == nullptr; } - MallocedBuffer() : data(nullptr), allocator(nullptr) {} - MallocedBuffer(size_t size, v8::ArrayBuffer::Allocator* allocator) - : size(size), allocator(allocator) { - data = static_cast(allocator->AllocateUninitialized(size)); - } - MallocedBuffer(char *data, size_t size, v8::ArrayBuffer::Allocator *allocator) - : data(data), size(size), allocator(allocator) {} - MallocedBuffer(MallocedBuffer&& other) - : data(other.data), size(other.size), allocator(other.allocator) { + MallocedBuffer() : data(nullptr) {} + explicit MallocedBuffer(size_t size) : data(Malloc(size)), size(size) {} + MallocedBuffer(char* data, size_t size) : data(data), size(size) {} + MallocedBuffer(MallocedBuffer&& other) : data(other.data), size(other.size) { other.data = nullptr; - other.allocator = nullptr; } MallocedBuffer& operator=(MallocedBuffer&& other) { this->~MallocedBuffer(); return *new(this) MallocedBuffer(std::move(other)); } ~MallocedBuffer() { - if (allocator) { - allocator->Free(data, size); - } else { - free(data); - } + free(data); } MallocedBuffer(const MallocedBuffer&) = delete; MallocedBuffer& operator=(const MallocedBuffer&) = delete; diff --git a/test/parallel/test-http2-max-session-memory-leak.js b/test/parallel/test-http2-max-session-memory-leak.js new file mode 100644 index 00000000000..b066ca80bc5 --- /dev/null +++ b/test/parallel/test-http2-max-session-memory-leak.js @@ -0,0 +1,46 @@ +'use strict'; +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); +const http2 = require('http2'); + +// Regression test for https://github.com/nodejs/node/issues/27416. +// Check that received data is accounted for correctly in the maxSessionMemory +// mechanism. + +const bodyLength = 8192; +const maxSessionMemory = 1; // 1 MB +const requestCount = 1000; + +const server = http2.createServer({ maxSessionMemory }); +server.on('stream', (stream) => { + stream.respond(); + stream.end(); +}); + +server.listen(common.mustCall(() => { + const client = http2.connect(`http://localhost:${server.address().port}`, { + maxSessionMemory + }); + + function request() { + return new Promise((resolve, reject) => { + const stream = client.request({ + ':method': 'POST', + 'content-length': bodyLength + }); + stream.on('error', reject); + stream.on('response', resolve); + stream.end('a'.repeat(bodyLength)); + }); + } + + (async () => { + for (let i = 0; i < requestCount; i++) { + await request(); + } + + client.close(); + server.close(); + })().then(common.mustCall()); +}));