Skip to content
This repository was archived by the owner on Mar 4, 2020. It is now read-only.

fix: remove array buffer allocator patch (electron-4.x) #110

Open
wants to merge 12 commits into
base: electron-node-v10.11.0-V8-6.9
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions doc/api/errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

<a id="ERR_BUFFER_CONTEXT_NOT_AVAILABLE"></a>
### 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.

<a id="ERR_BUFFER_OUT_OF_BOUNDS"></a>
### ERR_BUFFER_OUT_OF_BOUNDS

Expand Down
110 changes: 108 additions & 2 deletions src/env-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -661,6 +669,104 @@ inline IsolateData* Environment::isolate_data() const {
return isolate_data_;
}

inline char* Environment::AllocateUnchecked(size_t size) {
return static_cast<char*>(
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<v8::Object> New(Environment* env,
char* data,
size_t length,
bool uses_malloc);
}

inline v8::MaybeLocal<v8::Object> AllocatedBuffer::ToBuffer() {
CHECK_NOT_NULL(env_);
v8::MaybeLocal<v8::Object> obj = Buffer::New(env_, data(), size(), false);
if (!obj.IsEmpty()) release();
return obj;
}

inline v8::Local<v8::ArrayBuffer> 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);
}
Expand Down
31 changes: 26 additions & 5 deletions src/env.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

namespace node {

using v8::ArrayBuffer;
using v8::Context;
using v8::FunctionTemplate;
using v8::HandleScope;
Expand All @@ -36,11 +37,14 @@ void* Environment::kNodeContextTagPtr = const_cast<void*>(
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);

Expand Down Expand Up @@ -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<char*>(
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) {
Expand Down
55 changes: 51 additions & 4 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<PerIsolateOptions> 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)
Expand Down Expand Up @@ -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<PerIsolateOptions> options_;

Expand All @@ -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<v8::Object> ToBuffer();
inline v8::Local<v8::ArrayBuffer> 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 {
Expand Down Expand Up @@ -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);

Expand Down
78 changes: 76 additions & 2 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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();
}


Expand All @@ -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);
}


Expand Down
Loading