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
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
  • Loading branch information
@legendecas @targos
legendecas authored and targos committed Mar 13, 2023
1 parent d627164 commit 973287a
Show file tree
Hide file tree
Showing 27 changed files with 211 additions and 203 deletions.
10 changes: 5 additions & 5 deletions src/README.md
View file
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
View file
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
14 changes: 7 additions & 7 deletions src/dataqueue/queue.cc
View file
Expand Up @@ -874,14 +874,14 @@ class FdEntry final : public EntryImpl {
uv_fs_close(nullptr, &req, file, nullptr);
return nullptr;
}
Realm* realm = entry->env()->principal_realm();
return std::make_shared<ReaderImpl>(
BaseObjectPtr<fs::FileHandle>(
fs::FileHandle::New(entry->env()->GetBindingData<fs::BindingData>(
entry->env()->context()),
file,
Local<Object>(),
entry->start_,
entry->end_)),
BaseObjectPtr<fs::FileHandle>(fs::FileHandle::New(
realm->GetBindingData<fs::BindingData>(realm->context()),
file,
Local<Object>(),
entry->start_,
entry->end_)),
entry);
}

Expand Down
42 changes: 0 additions & 42 deletions src/env-inl.h
View file
Expand Up @@ -201,48 +201,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
View file
Expand Up @@ -545,7 +545,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 @@ -1018,7 +1019,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
View file
Expand Up @@ -587,25 +587,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 @@ -1124,8 +1105,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
21 changes: 9 additions & 12 deletions src/node_blob.cc
View file
Expand Up @@ -110,10 +110,10 @@ 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);
Expand Down Expand Up @@ -394,8 +394,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 @@ -418,8 +417,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 @@ -430,8 +428,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 @@ -477,8 +474,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 @@ -516,9 +513,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
View file
Expand Up @@ -111,7 +111,7 @@ class Blob : public BaseObject {

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
View file
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
View file
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

0 comments on commit 973287a

Please sign in to comment.