From 0a24ee1729fd95d08cb5dc9b5508626a5e8b3b35 Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Fri, 8 Mar 2019 16:19:33 +0100
Subject: [PATCH 1/6] 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.
---
src/api/environment.cc | 22 +++++++++++++---------
src/env-inl.h | 2 +-
src/env.cc | 3 ++-
src/env.h | 4 ++--
src/node.h | 25 ++++++++++++++++++++++++-
src/node_buffer.cc | 3 ++-
src/node_internals.h | 6 ++++--
7 files changed, 48 insertions(+), 17 deletions(-)
diff --git a/src/api/environment.cc b/src/api/environment.cc
index cd3e3aefc2ccfd..3deb3a3a6c4d0b 100644
--- a/src/api/environment.cc
+++ b/src/api/environment.cc
@@ -71,7 +71,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
@@ -84,14 +84,14 @@ 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;
}
@@ -99,14 +99,14 @@ void* DebuggingArrayBufferAllocator::AllocateUninitialized(size_t size) {
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);
@@ -149,11 +149,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) {
diff --git a/src/env-inl.h b/src/env-inl.h
index 2a560f71d24bc1..b0acf24bb8575c 100644
--- a/src/env-inl.h
+++ b/src/env-inl.h
@@ -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_;
}
diff --git a/src/env.cc b/src/env.cc
index bd44480184bc4a..7ad43441a31a78 100644
--- a/src/env.cc
+++ b/src/env.cc
@@ -77,7 +77,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_);
diff --git a/src/env.h b/src/env.h
index 5ab3e1baf33223..fc27e8ed21645c 100644
--- a/src/env.h
+++ b/src/env.h
@@ -407,7 +407,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)
@@ -442,7 +442,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_;
diff --git a/src/node.h b/src/node.h
index 3ba5cf92f349d2..40659c69808444 100644
--- a/src/node.h
+++ b/src/node.h
@@ -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))
@@ -210,8 +212,29 @@ NODE_EXTERN void Init(int* argc,
int* exec_argc,
const char*** exec_argv);
-class ArrayBufferAllocator;
+class NodeArrayBufferAllocator;
+
+// A 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 = true);
+
+ private:
+ virtual NodeArrayBufferAllocator* GetImpl() = 0;
+
+ friend class IsolateData;
+};
+// Legacy equivalents for ArrayBufferAllocator::Create().
NODE_EXTERN ArrayBufferAllocator* CreateArrayBufferAllocator();
NODE_EXTERN void FreeArrayBufferAllocator(ArrayBufferAllocator* allocator);
diff --git a/src/node_buffer.cc b/src/node_buffer.cc
index 10b8f4098f2f7b..baf3ff50106db7 100644
--- a/src/node_buffer.cc
+++ b/src/node_buffer.cc
@@ -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));
diff --git a/src/node_internals.h b/src/node_internals.h
index 166cfd9ea23615..30ece190291d65 100644
--- a/src/node_internals.h
+++ b/src/node_internals.h
@@ -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_; }
@@ -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;
From 8a0ff6cd324359225b160b5fb76b6b567f80e8fc Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Fri, 8 Mar 2019 16:55:40 +0100
Subject: [PATCH 2/6] embedding: make `NewIsolate()` API more flexible
Split the API up into its essential parts, namely setting up
the creation parameters for the Isolate, creating it, and performing
Node.js-specific customization afterwards.
---
src/api/environment.cc | 42 +++++++++++++++++++++++++++++-------------
src/node.h | 15 +++++++++++++++
2 files changed, 44 insertions(+), 13 deletions(-)
diff --git a/src/api/environment.cc b/src/api/environment.cc
index 3deb3a3a6c4d0b..9283e795b85907 100644
--- a/src/api/environment.cc
+++ b/src/api/environment.cc
@@ -164,30 +164,25 @@ void FreeArrayBufferAllocator(ArrayBufferAllocator* allocator) {
delete allocator;
}
-Isolate* NewIsolate(ArrayBufferAllocator* allocator, uv_loop_t* event_loop) {
- Isolate::CreateParams params;
- params.array_buffer_allocator = allocator;
+void SetIsolateCreateParams(Isolate::CreateParams* params,
+ ArrayBufferAllocator* allocator) {
+ if (allocator != nullptr)
+ params->array_buffer_allocator = allocator;
double total_memory = uv_get_total_memory();
if (total_memory > 0) {
// V8 defaults to 700MB or 1.4GB on 32 and 64 bit platforms respectively.
// This default is based on browser use-cases. Tell V8 to configure the
// heap based on the actual physical memory.
- params.constraints.ConfigureDefaults(total_memory, 0);
+ params->constraints.ConfigureDefaults(total_memory, 0);
}
#ifdef NODE_ENABLE_VTUNE_PROFILING
- params.code_event_handler = vTune::GetVtuneCodeEventHandler();
+ params->code_event_handler = vTune::GetVtuneCodeEventHandler();
#endif
+}
- Isolate* isolate = Isolate::Allocate();
- if (isolate == nullptr) return nullptr;
-
- // Register the isolate on the platform before the isolate gets initialized,
- // so that the isolate can access the platform during initialization.
- per_process::v8_platform.Platform()->RegisterIsolate(isolate, event_loop);
- Isolate::Initialize(isolate, params);
-
+void SetIsolateUpForNode(v8::Isolate* isolate) {
isolate->AddMessageListenerWithErrorLevel(
OnMessage,
Isolate::MessageErrorLevel::kMessageError |
@@ -197,6 +192,27 @@ Isolate* NewIsolate(ArrayBufferAllocator* allocator, uv_loop_t* event_loop) {
isolate->SetFatalErrorHandler(OnFatalError);
isolate->SetAllowWasmCodeGenerationCallback(AllowWasmCodeGenerationCallback);
v8::CpuProfiler::UseDetailedSourcePositionsForProfiling(isolate);
+}
+
+Isolate* NewIsolate(ArrayBufferAllocator* allocator, uv_loop_t* event_loop) {
+ return NewIsolate(allocator, event_loop, GetMainThreadMultiIsolatePlatform());
+}
+
+Isolate* NewIsolate(ArrayBufferAllocator* allocator,
+ uv_loop_t* event_loop,
+ MultiIsolatePlatform* platform) {
+ Isolate::CreateParams params;
+ SetIsolateCreateParams(¶ms, allocator);
+
+ Isolate* isolate = Isolate::Allocate();
+ if (isolate == nullptr) return nullptr;
+
+ // Register the isolate on the platform before the isolate gets initialized,
+ // so that the isolate can access the platform during initialization.
+ platform->RegisterIsolate(isolate, event_loop);
+ Isolate::Initialize(isolate, params);
+
+ SetIsolateUpForNode(isolate);
return isolate;
}
diff --git a/src/node.h b/src/node.h
index 40659c69808444..cfe6d0219d1877 100644
--- a/src/node.h
+++ b/src/node.h
@@ -257,9 +257,24 @@ class NODE_EXTERN MultiIsolatePlatform : public v8::Platform {
virtual void UnregisterIsolate(v8::Isolate* isolate) = 0;
};
+// Set up some Node.js-specific defaults for `params`, in particular
+// the ArrayBuffer::Allocator if it is provided, memory limits, and
+// possibly a code event handler.
+NODE_EXTERN void SetIsolateCreateParams(v8::Isolate::CreateParams* params,
+ ArrayBufferAllocator* allocator
+ = nullptr);
+// Set a number of callbacks for the `isolate`, in particular the Node.js
+// uncaught exception listener.
+NODE_EXTERN void SetIsolateUpForNode(v8::Isolate* isolate);
// Creates a new isolate with Node.js-specific settings.
+// This is a convenience method equivalent to using SetIsolateCreateParams(),
+// Isolate::Allocate(), MultiIsolatePlatform::RegisterIsolate(),
+// Isolate::Initialize(), and SetIsolateUpForNow().
NODE_EXTERN v8::Isolate* NewIsolate(ArrayBufferAllocator* allocator,
struct uv_loop_s* event_loop);
+NODE_EXTERN v8::Isolate* NewIsolate(ArrayBufferAllocator* allocator,
+ struct uv_loop_s* event_loop,
+ MultiIsolatePlatform* platform);
// Creates a new context with Node.js-specific tweaks.
NODE_EXTERN v8::Local<v8::Context> NewContext(
From ff05c1a31039796df65a4ee50ace99e20bbf8739 Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Fri, 8 Mar 2019 17:56:12 +0100
Subject: [PATCH 3/6] fixup! embedding: refactor public `ArrayBufferAllocator`
API
---
src/node.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/node.h b/src/node.h
index cfe6d0219d1877..f99be025c4bbd1 100644
--- a/src/node.h
+++ b/src/node.h
@@ -214,7 +214,7 @@ NODE_EXTERN void Init(int* argc,
class NodeArrayBufferAllocator;
-// A ArrayBuffer::Allocator class with some Node.js-specific tweaks. If you do
+// 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.
From 0f26254b416227de5b576a76eebfc0290d3979db Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Fri, 8 Mar 2019 18:24:32 +0100
Subject: [PATCH 4/6] fixup! embedding: make `NewIsolate()` API more flexible
---
src/api/environment.cc | 1 +
src/env.cc | 4 ----
2 files changed, 1 insertion(+), 4 deletions(-)
diff --git a/src/api/environment.cc b/src/api/environment.cc
index 9283e795b85907..a2d3b81bdb8e36 100644
--- a/src/api/environment.cc
+++ b/src/api/environment.cc
@@ -191,6 +191,7 @@ void SetIsolateUpForNode(v8::Isolate* isolate) {
isolate->SetMicrotasksPolicy(MicrotasksPolicy::kExplicit);
isolate->SetFatalErrorHandler(OnFatalError);
isolate->SetAllowWasmCodeGenerationCallback(AllowWasmCodeGenerationCallback);
+ isolate->SetPromiseRejectCallback(task_queue::PromiseRejectCallback);
v8::CpuProfiler::UseDetailedSourcePositionsForProfiling(isolate);
}
diff --git a/src/env.cc b/src/env.cc
index 7ad43441a31a78..5009b1d3b5631e 100644
--- a/src/env.cc
+++ b/src/env.cc
@@ -258,10 +258,6 @@ Environment::Environment(IsolateData* isolate_data,
if (options_->no_force_async_hooks_checks) {
async_hooks_.no_force_checks();
}
-
- // TODO(addaleax): the per-isolate state should not be controlled by
- // a single Environment.
- isolate()->SetPromiseRejectCallback(task_queue::PromiseRejectCallback);
}
CompileFnEntry::CompileFnEntry(Environment* env, uint32_t id)
From 45e1f65e2024e5e3a5773a704d8bcb012e8422c4 Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Sat, 9 Mar 2019 12:00:51 +0100
Subject: [PATCH 5/6] fixup! embedding: make `NewIsolate()` API more flexible
---
src/node.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/node.h b/src/node.h
index f99be025c4bbd1..1ec3c974c572ec 100644
--- a/src/node.h
+++ b/src/node.h
@@ -269,7 +269,7 @@ NODE_EXTERN void SetIsolateUpForNode(v8::Isolate* isolate);
// Creates a new isolate with Node.js-specific settings.
// This is a convenience method equivalent to using SetIsolateCreateParams(),
// Isolate::Allocate(), MultiIsolatePlatform::RegisterIsolate(),
-// Isolate::Initialize(), and SetIsolateUpForNow().
+// Isolate::Initialize(), and SetIsolateUpForNode().
NODE_EXTERN v8::Isolate* NewIsolate(ArrayBufferAllocator* allocator,
struct uv_loop_s* event_loop);
NODE_EXTERN v8::Isolate* NewIsolate(ArrayBufferAllocator* allocator,
From 30d1e83437b6db6b26345b45486361c7d9dde76e Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Wed, 13 Mar 2019 00:18:36 +0000
Subject: [PATCH 6/6] fixup! embedding: refactor public `ArrayBufferAllocator`
API
---
src/node.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/node.h b/src/node.h
index 1ec3c974c572ec..fecab208189554 100644
--- a/src/node.h
+++ b/src/node.h
@@ -226,7 +226,8 @@ class NODE_EXTERN ArrayBufferAllocator : public v8::ArrayBuffer::Allocator {
// 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 = true);
+ static std::unique_ptr<ArrayBufferAllocator> Create(
+ bool always_debug = false);
private:
virtual NodeArrayBufferAllocator* GetImpl() = 0;