Skip to content

Commit

Permalink
src: per-realm binding data
Browse files Browse the repository at this point in the history
Binding data is inherited from BaseObject and created in a specific
realm. They need to be tracked on a per-realm basis so that they can
be released properly when a realm is disposed.

PR-URL: #46556
Backport-PR-URL: #47174
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
# Conflicts:
#	src/dataqueue/queue.cc
  • Loading branch information
legendecas authored and RafaelGSS committed Jun 20, 2023
1 parent 9ca3cc0 commit 05f5c79
Show file tree
Hide file tree
Showing 26 changed files with 205 additions and 197 deletions.
10 changes: 5 additions & 5 deletions src/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -482,7 +482,7 @@ Which explains that the unregistered external reference is

Some internal bindings, such as the HTTP parser, maintain internal state that
only affects that particular binding. In that case, one common way to store
that state is through the use of `Environment::AddBindingData`, which gives
that state is through the use of `Realm::AddBindingData`, which gives
binding functions access to an object for storing such state.
That object is always a [`BaseObject`][].

Expand All @@ -507,7 +507,7 @@ class BindingData : public BaseObject {

// Available for binding functions, e.g. the HTTP Parser constructor:
static void New(const FunctionCallbackInfo<Value>& args) {
BindingData* binding_data = Environment::GetBindingData<BindingData>(args);
BindingData* binding_data = Realm::GetBindingData<BindingData>(args);
new Parser(binding_data, args.This());
}

Expand All @@ -517,12 +517,12 @@ void InitializeHttpParser(Local<Object> target,
Local<Value> unused,
Local<Context> context,
void* priv) {
Environment* env = Environment::GetCurrent(context);
Realm* realm = Realm::GetCurrent(context);
BindingData* const binding_data =
env->AddBindingData<BindingData>(context, target);
realm->AddBindingData<BindingData>(context, target);
if (binding_data == nullptr) return;

Local<FunctionTemplate> t = env->NewFunctionTemplate(Parser::New);
Local<FunctionTemplate> t = NewFunctionTemplate(realm->isolate(), Parser::New);
...
}
```
Expand Down
5 changes: 4 additions & 1 deletion src/base_object-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,10 @@
namespace node {

BaseObject::BaseObject(Environment* env, v8::Local<v8::Object> object)
: BaseObject(env->principal_realm(), object) {}
: BaseObject(env->principal_realm(), object) {
// TODO(legendecas): Check the shorthand is only used in the principal realm
// while allowing to create a BaseObject in a vm context.
}

// static
v8::Local<v8::FunctionTemplate> BaseObject::GetConstructorTemplate(
Expand Down
42 changes: 0 additions & 42 deletions src/env-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -197,48 +197,6 @@ inline Environment* Environment::GetCurrent(
return GetCurrent(info.GetIsolate()->GetCurrentContext());
}

template <typename T, typename U>
inline T* Environment::GetBindingData(const v8::PropertyCallbackInfo<U>& info) {
return GetBindingData<T>(info.GetIsolate()->GetCurrentContext());
}

template <typename T>
inline T* Environment::GetBindingData(
const v8::FunctionCallbackInfo<v8::Value>& info) {
return GetBindingData<T>(info.GetIsolate()->GetCurrentContext());
}

template <typename T>
inline T* Environment::GetBindingData(v8::Local<v8::Context> context) {
BindingDataStore* map = static_cast<BindingDataStore*>(
context->GetAlignedPointerFromEmbedderData(
ContextEmbedderIndex::kBindingListIndex));
DCHECK_NOT_NULL(map);
auto it = map->find(T::type_name);
if (UNLIKELY(it == map->end())) return nullptr;
T* result = static_cast<T*>(it->second.get());
DCHECK_NOT_NULL(result);
DCHECK_EQ(result->env(), GetCurrent(context));
return result;
}

template <typename T>
inline T* Environment::AddBindingData(
v8::Local<v8::Context> context,
v8::Local<v8::Object> target) {
DCHECK_EQ(GetCurrent(context), this);
// This won't compile if T is not a BaseObject subclass.
BaseObjectPtr<T> item = MakeDetachedBaseObject<T>(this, target);
BindingDataStore* map = static_cast<BindingDataStore*>(
context->GetAlignedPointerFromEmbedderData(
ContextEmbedderIndex::kBindingListIndex));
DCHECK_NOT_NULL(map);
auto result = map->emplace(T::type_name, item);
CHECK(result.second);
DCHECK_EQ(GetBindingData<T>(context), item.get());
return item.get();
}

inline v8::Isolate* Environment::isolate() const {
return isolate_;
}
Expand Down
4 changes: 2 additions & 2 deletions src/env.cc
Original file line number Diff line number Diff line change
Expand Up @@ -539,7 +539,8 @@ void Environment::AssignToContext(Local<v8::Context> context,
context->SetAlignedPointerInEmbedderData(ContextEmbedderIndex::kRealm, realm);
// Used to retrieve bindings
context->SetAlignedPointerInEmbedderData(
ContextEmbedderIndex::kBindingListIndex, &(this->bindings_));
ContextEmbedderIndex::kBindingDataStoreIndex,
realm->binding_data_store());

// ContextifyContexts will update this to a pointer to the native object.
context->SetAlignedPointerInEmbedderData(
Expand Down Expand Up @@ -1010,7 +1011,6 @@ MaybeLocal<Value> Environment::RunSnapshotDeserializeMain() const {
void Environment::RunCleanup() {
started_cleanup_ = true;
TRACE_EVENT0(TRACING_CATEGORY_NODE1(environment), "RunCleanup");
bindings_.clear();
// Only BaseObject's cleanups are registered as per-realm cleanup hooks now.
// Defer the BaseObject cleanup after handles are cleaned up.
CleanupHandles();
Expand Down
21 changes: 0 additions & 21 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -571,25 +571,6 @@ class Environment : public MemoryRetainer {
static inline Environment* GetCurrent(
const v8::PropertyCallbackInfo<T>& info);

// Methods created using SetMethod(), SetPrototypeMethod(), etc. inside
// this scope can access the created T* object using
// GetBindingData<T>(args) later.
template <typename T>
T* AddBindingData(v8::Local<v8::Context> context,
v8::Local<v8::Object> target);
template <typename T, typename U>
static inline T* GetBindingData(const v8::PropertyCallbackInfo<U>& info);
template <typename T>
static inline T* GetBindingData(
const v8::FunctionCallbackInfo<v8::Value>& info);
template <typename T>
static inline T* GetBindingData(v8::Local<v8::Context> context);

typedef std::unordered_map<
FastStringKey,
BaseObjectPtr<BaseObject>,
FastStringKey::Hash> BindingDataStore;

// Create an Environment without initializing a main Context. Use
// InitializeMainContext() to initialize a main context for it.
Environment(IsolateData* isolate_data,
Expand Down Expand Up @@ -1108,8 +1089,6 @@ class Environment : public MemoryRetainer {
void RequestInterruptFromV8();
static void CheckImmediate(uv_check_t* handle);

BindingDataStore bindings_;

CleanupQueue cleanup_queue_;
bool started_cleanup_ = false;

Expand Down
23 changes: 10 additions & 13 deletions src/node_blob.cc
Original file line number Diff line number Diff line change
Expand Up @@ -37,17 +37,17 @@ void Blob::Initialize(
Local<Value> unused,
Local<Context> context,
void* priv) {
Environment* env = Environment::GetCurrent(context);
Realm* realm = Realm::GetCurrent(context);

BlobBindingData* const binding_data =
env->AddBindingData<BlobBindingData>(context, target);
realm->AddBindingData<BlobBindingData>(context, target);
if (binding_data == nullptr) return;

SetMethod(context, target, "createBlob", New);
SetMethod(context, target, "storeDataObject", StoreDataObject);
SetMethod(context, target, "getDataObject", GetDataObject);
SetMethod(context, target, "revokeDataObject", RevokeDataObject);
FixedSizeBlobCopyJob::Initialize(env, target);
FixedSizeBlobCopyJob::Initialize(realm->env(), target);
}

Local<FunctionTemplate> Blob::GetConstructorTemplate(Environment* env) {
Expand Down Expand Up @@ -236,8 +236,7 @@ std::unique_ptr<worker::TransferData> Blob::CloneForMessaging() const {

void Blob::StoreDataObject(const v8::FunctionCallbackInfo<v8::Value>& args) {
Environment* env = Environment::GetCurrent(args);
BlobBindingData* binding_data =
Environment::GetBindingData<BlobBindingData>(args);
BlobBindingData* binding_data = Realm::GetBindingData<BlobBindingData>(args);

CHECK(args[0]->IsString()); // ID key
CHECK(Blob::HasInstance(env, args[1])); // Blob
Expand All @@ -260,8 +259,7 @@ void Blob::StoreDataObject(const v8::FunctionCallbackInfo<v8::Value>& args) {
}

void Blob::RevokeDataObject(const v8::FunctionCallbackInfo<v8::Value>& args) {
BlobBindingData* binding_data =
Environment::GetBindingData<BlobBindingData>(args);
BlobBindingData* binding_data = Realm::GetBindingData<BlobBindingData>(args);

Environment* env = Environment::GetCurrent(args);
CHECK(args[0]->IsString()); // ID key
Expand All @@ -272,8 +270,7 @@ void Blob::RevokeDataObject(const v8::FunctionCallbackInfo<v8::Value>& args) {
}

void Blob::GetDataObject(const v8::FunctionCallbackInfo<v8::Value>& args) {
BlobBindingData* binding_data =
Environment::GetBindingData<BlobBindingData>(args);
BlobBindingData* binding_data = Realm::GetBindingData<BlobBindingData>(args);

Environment* env = Environment::GetCurrent(args);
CHECK(args[0]->IsString());
Expand Down Expand Up @@ -428,8 +425,8 @@ BlobBindingData::StoredDataObject::StoredDataObject(
length(length_),
type(type_) {}

BlobBindingData::BlobBindingData(Environment* env, Local<Object> wrap)
: SnapshotableObject(env, wrap, type_int) {
BlobBindingData::BlobBindingData(Realm* realm, Local<Object> wrap)
: SnapshotableObject(realm, wrap, type_int) {
MakeWeak();
}

Expand Down Expand Up @@ -465,9 +462,9 @@ void BlobBindingData::Deserialize(Local<Context> context,
InternalFieldInfoBase* info) {
DCHECK_EQ(index, BaseObject::kEmbedderType);
HandleScope scope(context->GetIsolate());
Environment* env = Environment::GetCurrent(context);
Realm* realm = Realm::GetCurrent(context);
BlobBindingData* binding =
env->AddBindingData<BlobBindingData>(context, holder);
realm->AddBindingData<BlobBindingData>(context, holder);
CHECK_NOT_NULL(binding);
}

Expand Down
2 changes: 1 addition & 1 deletion src/node_blob.h
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ class FixedSizeBlobCopyJob : public AsyncWrap, public ThreadPoolWork {

class BlobBindingData : public SnapshotableObject {
public:
explicit BlobBindingData(Environment* env, v8::Local<v8::Object> wrap);
explicit BlobBindingData(Realm* realm, v8::Local<v8::Object> wrap);

using InternalFieldInfo = InternalFieldInfoBase;

Expand Down
6 changes: 3 additions & 3 deletions src/node_context_data.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ namespace node {
#define NODE_CONTEXT_ALLOW_WASM_CODE_GENERATION_INDEX 34
#endif

#ifndef NODE_BINDING_LIST
#define NODE_BINDING_LIST_INDEX 35
#ifndef NODE_BINDING_DATA_STORE_INDEX
#define NODE_BINDING_DATA_STORE_INDEX 35
#endif

#ifndef NODE_CONTEXT_ALLOW_CODE_GENERATION_FROM_STRINGS_INDEX
Expand All @@ -51,7 +51,7 @@ enum ContextEmbedderIndex {
kEnvironment = NODE_CONTEXT_EMBEDDER_DATA_INDEX,
kSandboxObject = NODE_CONTEXT_SANDBOX_OBJECT_INDEX,
kAllowWasmCodeGeneration = NODE_CONTEXT_ALLOW_WASM_CODE_GENERATION_INDEX,
kBindingListIndex = NODE_BINDING_LIST_INDEX,
kBindingDataStoreIndex = NODE_BINDING_DATA_STORE_INDEX,
kAllowCodeGenerationFromStrings =
NODE_CONTEXT_ALLOW_CODE_GENERATION_FROM_STRINGS_INDEX,
kContextifyContext = NODE_CONTEXT_CONTEXTIFY_CONTEXT_INDEX,
Expand Down
2 changes: 1 addition & 1 deletion src/node_file-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ FSReqBase* GetReqWrap(const v8::FunctionCallbackInfo<v8::Value>& args,
return Unwrap<FSReqBase>(value.As<v8::Object>());
}

BindingData* binding_data = Environment::GetBindingData<BindingData>(args);
BindingData* binding_data = Realm::GetBindingData<BindingData>(args);
Environment* env = binding_data->env();
if (value->StrictEquals(env->fs_use_promises_symbol())) {
if (use_bigint) {
Expand Down
Loading

0 comments on commit 05f5c79

Please sign in to comment.