Skip to content

Commit 7671a65

Browse files
addaleaxtargos
authored andcommittedMar 30, 2019
embedding: refactor public ArrayBufferAllocator API
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>
1 parent c51cc9e commit 7671a65

File tree

7 files changed

+49
-17
lines changed

7 files changed

+49
-17
lines changed
 

‎src/api/environment.cc

+13-9
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ static void OnMessage(Local<Message> message, Local<Value> error) {
7171
}
7272
}
7373

74-
void* ArrayBufferAllocator::Allocate(size_t size) {
74+
void* NodeArrayBufferAllocator::Allocate(size_t size) {
7575
if (zero_fill_field_ || per_process::cli_options->zero_fill_all_buffers)
7676
return UncheckedCalloc(size);
7777
else
@@ -84,29 +84,29 @@ DebuggingArrayBufferAllocator::~DebuggingArrayBufferAllocator() {
8484

8585
void* DebuggingArrayBufferAllocator::Allocate(size_t size) {
8686
Mutex::ScopedLock lock(mutex_);
87-
void* data = ArrayBufferAllocator::Allocate(size);
87+
void* data = NodeArrayBufferAllocator::Allocate(size);
8888
RegisterPointerInternal(data, size);
8989
return data;
9090
}
9191

9292
void* DebuggingArrayBufferAllocator::AllocateUninitialized(size_t size) {
9393
Mutex::ScopedLock lock(mutex_);
94-
void* data = ArrayBufferAllocator::AllocateUninitialized(size);
94+
void* data = NodeArrayBufferAllocator::AllocateUninitialized(size);
9595
RegisterPointerInternal(data, size);
9696
return data;
9797
}
9898

9999
void DebuggingArrayBufferAllocator::Free(void* data, size_t size) {
100100
Mutex::ScopedLock lock(mutex_);
101101
UnregisterPointerInternal(data, size);
102-
ArrayBufferAllocator::Free(data, size);
102+
NodeArrayBufferAllocator::Free(data, size);
103103
}
104104

105105
void* DebuggingArrayBufferAllocator::Reallocate(void* data,
106106
size_t old_size,
107107
size_t size) {
108108
Mutex::ScopedLock lock(mutex_);
109-
void* ret = ArrayBufferAllocator::Reallocate(data, old_size, size);
109+
void* ret = NodeArrayBufferAllocator::Reallocate(data, old_size, size);
110110
if (ret == nullptr) {
111111
if (size == 0) // i.e. equivalent to free().
112112
UnregisterPointerInternal(data, old_size);
@@ -149,11 +149,15 @@ void DebuggingArrayBufferAllocator::RegisterPointerInternal(void* data,
149149
allocations_[data] = size;
150150
}
151151

152-
ArrayBufferAllocator* CreateArrayBufferAllocator() {
153-
if (per_process::cli_options->debug_arraybuffer_allocations)
154-
return new DebuggingArrayBufferAllocator();
152+
std::unique_ptr<ArrayBufferAllocator> ArrayBufferAllocator::Create(bool debug) {
153+
if (debug || per_process::cli_options->debug_arraybuffer_allocations)
154+
return std::make_unique<DebuggingArrayBufferAllocator>();
155155
else
156-
return new ArrayBufferAllocator();
156+
return std::make_unique<NodeArrayBufferAllocator>();
157+
}
158+
159+
ArrayBufferAllocator* CreateArrayBufferAllocator() {
160+
return ArrayBufferAllocator::Create().release();
157161
}
158162

159163
void FreeArrayBufferAllocator(ArrayBufferAllocator* allocator) {

‎src/env-inl.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ inline v8::ArrayBuffer::Allocator* IsolateData::allocator() const {
5656
return allocator_;
5757
}
5858

59-
inline ArrayBufferAllocator* IsolateData::node_allocator() const {
59+
inline NodeArrayBufferAllocator* IsolateData::node_allocator() const {
6060
return node_allocator_;
6161
}
6262

‎src/env.cc

+2-1
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,8 @@ IsolateData::IsolateData(Isolate* isolate,
8080
: isolate_(isolate),
8181
event_loop_(event_loop),
8282
allocator_(isolate->GetArrayBufferAllocator()),
83-
node_allocator_(node_allocator),
83+
node_allocator_(node_allocator == nullptr ?
84+
nullptr : node_allocator->GetImpl()),
8485
uses_node_allocator_(allocator_ == node_allocator_),
8586
platform_(platform) {
8687
CHECK_NOT_NULL(allocator_);

‎src/env.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -408,7 +408,7 @@ class IsolateData {
408408

409409
inline bool uses_node_allocator() const;
410410
inline v8::ArrayBuffer::Allocator* allocator() const;
411-
inline ArrayBufferAllocator* node_allocator() const;
411+
inline NodeArrayBufferAllocator* node_allocator() const;
412412

413413
#define VP(PropertyName, StringValue) V(v8::Private, PropertyName)
414414
#define VY(PropertyName, StringValue) V(v8::Symbol, PropertyName)
@@ -445,7 +445,7 @@ class IsolateData {
445445
v8::Isolate* const isolate_;
446446
uv_loop_t* const event_loop_;
447447
v8::ArrayBuffer::Allocator* const allocator_;
448-
ArrayBufferAllocator* const node_allocator_;
448+
NodeArrayBufferAllocator* const node_allocator_;
449449
const bool uses_node_allocator_;
450450
MultiIsolatePlatform* platform_;
451451
std::shared_ptr<PerIsolateOptions> options_;

‎src/node.h

+25-1
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,8 @@
6464
#include "v8-platform.h" // NOLINT(build/include_order)
6565
#include "node_version.h" // NODE_MODULE_VERSION
6666

67+
#include <memory>
68+
6769
#define NODE_MAKE_VERSION(major, minor, patch) \
6870
((major) * 0x1000 + (minor) * 0x100 + (patch))
6971

@@ -217,8 +219,30 @@ NODE_EXTERN void Init(int* argc,
217219
int* exec_argc,
218220
const char*** exec_argv);
219221

220-
class ArrayBufferAllocator;
222+
class NodeArrayBufferAllocator;
223+
224+
// An ArrayBuffer::Allocator class with some Node.js-specific tweaks. If you do
225+
// not have to use another allocator, using this class is recommended:
226+
// - It supports Buffer.allocUnsafe() and Buffer.allocUnsafeSlow() with
227+
// uninitialized memory.
228+
// - It supports transferring, rather than copying, ArrayBuffers when using
229+
// MessagePorts.
230+
class NODE_EXTERN ArrayBufferAllocator : public v8::ArrayBuffer::Allocator {
231+
public:
232+
// If `always_debug` is true, create an ArrayBuffer::Allocator instance
233+
// that performs additional integrity checks (e.g. make sure that only memory
234+
// that was allocated by the it is also freed by it).
235+
// This can also be set using the --debug-arraybuffer-allocations flag.
236+
static std::unique_ptr<ArrayBufferAllocator> Create(
237+
bool always_debug = false);
238+
239+
private:
240+
virtual NodeArrayBufferAllocator* GetImpl() = 0;
241+
242+
friend class IsolateData;
243+
};
221244

245+
// Legacy equivalents for ArrayBufferAllocator::Create().
222246
NODE_EXTERN ArrayBufferAllocator* CreateArrayBufferAllocator();
223247
NODE_EXTERN void FreeArrayBufferAllocator(ArrayBufferAllocator* allocator);
224248

‎src/node_buffer.cc

+2-1
Original file line numberDiff line numberDiff line change
@@ -1099,7 +1099,8 @@ void Initialize(Local<Object> target,
10991099

11001100
// It can be a nullptr when running inside an isolate where we
11011101
// do not own the ArrayBuffer allocator.
1102-
if (ArrayBufferAllocator* allocator = env->isolate_data()->node_allocator()) {
1102+
if (NodeArrayBufferAllocator* allocator =
1103+
env->isolate_data()->node_allocator()) {
11031104
uint32_t* zero_fill_field = allocator->zero_fill_field();
11041105
Local<ArrayBuffer> array_buffer = ArrayBuffer::New(
11051106
env->isolate(), zero_fill_field, sizeof(*zero_fill_field));

‎src/node_internals.h

+4-2
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ namespace task_queue {
101101
void PromiseRejectCallback(v8::PromiseRejectMessage message);
102102
} // namespace task_queue
103103

104-
class ArrayBufferAllocator : public v8::ArrayBuffer::Allocator {
104+
class NodeArrayBufferAllocator : public ArrayBufferAllocator {
105105
public:
106106
inline uint32_t* zero_fill_field() { return &zero_fill_field_; }
107107

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

119+
NodeArrayBufferAllocator* GetImpl() final { return this; }
120+
119121
private:
120122
uint32_t zero_fill_field_ = 1; // Boolean but exposed as uint32 to JS land.
121123
};
122124

123-
class DebuggingArrayBufferAllocator final : public ArrayBufferAllocator {
125+
class DebuggingArrayBufferAllocator final : public NodeArrayBufferAllocator {
124126
public:
125127
~DebuggingArrayBufferAllocator() override;
126128
void* Allocate(size_t size) override;

0 commit comments

Comments
 (0)
Please sign in to comment.