Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

src: per-realm binding data #46556

Merged
merged 1 commit into from
Feb 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
14 changes: 7 additions & 7 deletions src/dataqueue/queue.cc
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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 @@ -1019,7 +1020,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 @@ -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 @@ -1134,8 +1115,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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
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