Skip to content

Commit

Permalink
embedding: refactor public ArrayBufferAllocator API
Browse files Browse the repository at this point in the history
Use a RAII approach by default, and make it possible for embedders
to use the `ArrayBufferAllocator` directly as a V8
`ArrayBuffer::Allocator`, e.g. when passing to `Isolate::CreateParams`
manually.

PR-URL: #26525
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
  • Loading branch information
addaleax committed Mar 13, 2019
1 parent 17ab2ed commit 0e3addd
Show file tree
Hide file tree
Showing 7 changed files with 49 additions and 17 deletions.
22 changes: 13 additions & 9 deletions src/api/environment.cc
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ static void OnMessage(Local<Message> message, Local<Value> error) {
}
}

void* ArrayBufferAllocator::Allocate(size_t size) {
void* NodeArrayBufferAllocator::Allocate(size_t size) {
if (zero_fill_field_ || per_process::cli_options->zero_fill_all_buffers)
return UncheckedCalloc(size);
else
Expand All @@ -81,29 +81,29 @@ DebuggingArrayBufferAllocator::~DebuggingArrayBufferAllocator() {

void* DebuggingArrayBufferAllocator::Allocate(size_t size) {
Mutex::ScopedLock lock(mutex_);
void* data = ArrayBufferAllocator::Allocate(size);
void* data = NodeArrayBufferAllocator::Allocate(size);
RegisterPointerInternal(data, size);
return data;
}

void* DebuggingArrayBufferAllocator::AllocateUninitialized(size_t size) {
Mutex::ScopedLock lock(mutex_);
void* data = ArrayBufferAllocator::AllocateUninitialized(size);
void* data = NodeArrayBufferAllocator::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);
NodeArrayBufferAllocator::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);
void* ret = NodeArrayBufferAllocator::Reallocate(data, old_size, size);
if (ret == nullptr) {
if (size == 0) // i.e. equivalent to free().
UnregisterPointerInternal(data, old_size);
Expand Down Expand Up @@ -146,11 +146,15 @@ void DebuggingArrayBufferAllocator::RegisterPointerInternal(void* data,
allocations_[data] = size;
}

ArrayBufferAllocator* CreateArrayBufferAllocator() {
if (per_process::cli_options->debug_arraybuffer_allocations)
return new DebuggingArrayBufferAllocator();
std::unique_ptr<ArrayBufferAllocator> ArrayBufferAllocator::Create(bool debug) {
if (debug || per_process::cli_options->debug_arraybuffer_allocations)
return std::make_unique<DebuggingArrayBufferAllocator>();
else
return new ArrayBufferAllocator();
return std::make_unique<NodeArrayBufferAllocator>();
}

ArrayBufferAllocator* CreateArrayBufferAllocator() {
return ArrayBufferAllocator::Create().release();
}

void FreeArrayBufferAllocator(ArrayBufferAllocator* allocator) {
Expand Down
2 changes: 1 addition & 1 deletion src/env-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ inline v8::ArrayBuffer::Allocator* IsolateData::allocator() const {
return allocator_;
}

inline ArrayBufferAllocator* IsolateData::node_allocator() const {
inline NodeArrayBufferAllocator* IsolateData::node_allocator() const {
return node_allocator_;
}

Expand Down
3 changes: 2 additions & 1 deletion src/env.cc
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,8 @@ IsolateData::IsolateData(Isolate* isolate,
: isolate_(isolate),
event_loop_(event_loop),
allocator_(isolate->GetArrayBufferAllocator()),
node_allocator_(node_allocator),
node_allocator_(node_allocator == nullptr ?
nullptr : node_allocator->GetImpl()),
uses_node_allocator_(allocator_ == node_allocator_),
platform_(platform) {
CHECK_NOT_NULL(allocator_);
Expand Down
4 changes: 2 additions & 2 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -408,7 +408,7 @@ class IsolateData {

inline bool uses_node_allocator() const;
inline v8::ArrayBuffer::Allocator* allocator() const;
inline ArrayBufferAllocator* node_allocator() const;
inline NodeArrayBufferAllocator* node_allocator() const;

#define VP(PropertyName, StringValue) V(v8::Private, PropertyName)
#define VY(PropertyName, StringValue) V(v8::Symbol, PropertyName)
Expand Down Expand Up @@ -443,7 +443,7 @@ class IsolateData {
v8::Isolate* const isolate_;
uv_loop_t* const event_loop_;
v8::ArrayBuffer::Allocator* const allocator_;
ArrayBufferAllocator* const node_allocator_;
NodeArrayBufferAllocator* const node_allocator_;
const bool uses_node_allocator_;
MultiIsolatePlatform* platform_;
std::shared_ptr<PerIsolateOptions> options_;
Expand Down
26 changes: 25 additions & 1 deletion src/node.h
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@
#include "v8-platform.h" // NOLINT(build/include_order)
#include "node_version.h" // NODE_MODULE_VERSION

#include <memory>

#define NODE_MAKE_VERSION(major, minor, patch) \
((major) * 0x1000 + (minor) * 0x100 + (patch))

Expand Down Expand Up @@ -210,8 +212,30 @@ NODE_EXTERN void Init(int* argc,
int* exec_argc,
const char*** exec_argv);

class ArrayBufferAllocator;
class NodeArrayBufferAllocator;

// An ArrayBuffer::Allocator class with some Node.js-specific tweaks. If you do
// not have to use another allocator, using this class is recommended:
// - It supports Buffer.allocUnsafe() and Buffer.allocUnsafeSlow() with
// uninitialized memory.
// - It supports transferring, rather than copying, ArrayBuffers when using
// MessagePorts.
class NODE_EXTERN ArrayBufferAllocator : public v8::ArrayBuffer::Allocator {
public:
// If `always_debug` is true, create an ArrayBuffer::Allocator instance
// that performs additional integrity checks (e.g. make sure that only memory
// that was allocated by the it is also freed by it).
// This can also be set using the --debug-arraybuffer-allocations flag.
static std::unique_ptr<ArrayBufferAllocator> Create(
bool always_debug = false);

private:
virtual NodeArrayBufferAllocator* GetImpl() = 0;

friend class IsolateData;
};

// Legacy equivalents for ArrayBufferAllocator::Create().
NODE_EXTERN ArrayBufferAllocator* CreateArrayBufferAllocator();
NODE_EXTERN void FreeArrayBufferAllocator(ArrayBufferAllocator* allocator);

Expand Down
3 changes: 2 additions & 1 deletion src/node_buffer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1104,7 +1104,8 @@ void Initialize(Local<Object> target,

// It can be a nullptr when running inside an isolate where we
// do not own the ArrayBuffer allocator.
if (ArrayBufferAllocator* allocator = env->isolate_data()->node_allocator()) {
if (NodeArrayBufferAllocator* allocator =
env->isolate_data()->node_allocator()) {
uint32_t* zero_fill_field = allocator->zero_fill_field();
Local<ArrayBuffer> array_buffer = ArrayBuffer::New(
env->isolate(), zero_fill_field, sizeof(*zero_fill_field));
Expand Down
6 changes: 4 additions & 2 deletions src/node_internals.h
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ namespace task_queue {
void PromiseRejectCallback(v8::PromiseRejectMessage message);
} // namespace task_queue

class ArrayBufferAllocator : public v8::ArrayBuffer::Allocator {
class NodeArrayBufferAllocator : public ArrayBufferAllocator {
public:
inline uint32_t* zero_fill_field() { return &zero_fill_field_; }

Expand All @@ -116,11 +116,13 @@ class ArrayBufferAllocator : public v8::ArrayBuffer::Allocator {
virtual void RegisterPointer(void* data, size_t size) {}
virtual void UnregisterPointer(void* data, size_t size) {}

NodeArrayBufferAllocator* GetImpl() final { return this; }

private:
uint32_t zero_fill_field_ = 1; // Boolean but exposed as uint32 to JS land.
};

class DebuggingArrayBufferAllocator final : public ArrayBufferAllocator {
class DebuggingArrayBufferAllocator final : public NodeArrayBufferAllocator {
public:
~DebuggingArrayBufferAllocator() override;
void* Allocate(size_t size) override;
Expand Down

0 comments on commit 0e3addd

Please sign in to comment.