Skip to content

Commit

Permalink
src: make creating per-binding data structures easier
Browse files Browse the repository at this point in the history
Enable the state associated with the individual bindings, e.g. fs or
http2, to be moved out of the Environment class, in order for these
to be more modular and for Environment to be come less of a collection
of random data fields.

Do this by using a BaseObject as the data for callbacks, which can hold
the per-binding state. By default, no per-binding state is available,
although that can be configured when setting up the binding.

PR-URL: #32538
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
addaleax committed Apr 6, 2020
1 parent c2aedd0 commit 6576b9b
Show file tree
Hide file tree
Showing 12 changed files with 104 additions and 31 deletions.
7 changes: 4 additions & 3 deletions src/base_object-inl.h
Expand Up @@ -100,15 +100,16 @@ Environment* BaseObject::env() const {
return env_;
}

BaseObject* BaseObject::FromJSObject(v8::Local<v8::Object> obj) {
CHECK_GT(obj->InternalFieldCount(), 0);
BaseObject* BaseObject::FromJSObject(v8::Local<v8::Value> value) {
v8::Local<v8::Object> obj = value.As<v8::Object>();
DCHECK_GE(obj->InternalFieldCount(), BaseObject::kSlot);
return static_cast<BaseObject*>(
obj->GetAlignedPointerFromInternalField(BaseObject::kSlot));
}


template <typename T>
T* BaseObject::FromJSObject(v8::Local<v8::Object> object) {
T* BaseObject::FromJSObject(v8::Local<v8::Value> object) {
return static_cast<T*>(FromJSObject(object));
}

Expand Down
6 changes: 3 additions & 3 deletions src/base_object.h
Expand Up @@ -61,9 +61,9 @@ class BaseObject : public MemoryRetainer {
// was also passed to the `BaseObject()` constructor initially.
// This may return `nullptr` if the C++ object has not been constructed yet,
// e.g. when the JS object used `MakeLazilyInitializedJSTemplate`.
static inline BaseObject* FromJSObject(v8::Local<v8::Object> object);
static inline BaseObject* FromJSObject(v8::Local<v8::Value> object);
template <typename T>
static inline T* FromJSObject(v8::Local<v8::Object> object);
static inline T* FromJSObject(v8::Local<v8::Value> object);

// Make the `v8::Global` a weak reference and, `delete` this object once
// the JS object has been garbage collected and there are no (strong)
Expand Down Expand Up @@ -152,7 +152,7 @@ class BaseObject : public MemoryRetainer {

// Global alias for FromJSObject() to avoid churn.
template <typename T>
inline T* Unwrap(v8::Local<v8::Object> obj) {
inline T* Unwrap(v8::Local<v8::Value> obj) {
return BaseObject::FromJSObject<T>(obj);
}

Expand Down
48 changes: 42 additions & 6 deletions src/env-inl.h
Expand Up @@ -327,16 +327,48 @@ inline Environment* Environment::GetCurrent(
return GetFromCallbackData(info.Data());
}

inline Environment* Environment::GetFromCallbackData(v8::Local<v8::Value> val) {
Environment* Environment::GetFromCallbackData(v8::Local<v8::Value> val) {
DCHECK(val->IsObject());
v8::Local<v8::Object> obj = val.As<v8::Object>();
DCHECK_GE(obj->InternalFieldCount(), 1);
Environment* env =
static_cast<Environment*>(obj->GetAlignedPointerFromInternalField(0));
DCHECK_GE(obj->InternalFieldCount(),
BaseObject::kInternalFieldCount);
Environment* env = Unwrap<BaseObject>(obj)->env();
DCHECK(env->as_callback_data_template()->HasInstance(obj));
return env;
}

template <typename T>
Environment::BindingScope<T>::BindingScope(Environment* env) : env(env) {
v8::Local<v8::Object> callback_data;
if (!env->MakeBindingCallbackData<T>().ToLocal(&callback_data))
return;
data = Unwrap<T>(callback_data);

// No nesting allowed currently.
CHECK_EQ(env->current_callback_data(), env->as_callback_data());
env->set_current_callback_data(callback_data);
}

template <typename T>
Environment::BindingScope<T>::~BindingScope() {
env->set_current_callback_data(env->as_callback_data());
}

template <typename T>
v8::MaybeLocal<v8::Object> Environment::MakeBindingCallbackData() {
v8::Local<v8::Function> ctor;
v8::Local<v8::Object> obj;
if (!as_callback_data_template()->GetFunction(context()).ToLocal(&ctor) ||
!ctor->NewInstance(context()).ToLocal(&obj)) {
return v8::MaybeLocal<v8::Object>();
}
T* data = new T(this, obj);
// This won't compile if T is not a BaseObject subclass.
CHECK_EQ(data, static_cast<BaseObject*>(data));
data->MakeWeak();
return obj;
}

inline Environment* Environment::GetThreadLocalEnv() {
return static_cast<Environment*>(uv_key_get(&thread_local_env));
}
Expand Down Expand Up @@ -1123,7 +1155,7 @@ inline v8::Local<v8::FunctionTemplate>
v8::Local<v8::Signature> signature,
v8::ConstructorBehavior behavior,
v8::SideEffectType side_effect_type) {
v8::Local<v8::Object> external = as_callback_data();
v8::Local<v8::Object> external = current_callback_data();
return v8::FunctionTemplate::New(isolate(), callback, external,
signature, 0, behavior, side_effect_type);
}
Expand Down Expand Up @@ -1261,7 +1293,7 @@ void Environment::modify_base_object_count(int64_t delta) {
}

int64_t Environment::base_object_count() const {
return base_object_count_;
return base_object_count_ - initial_base_object_count_;
}

void Environment::set_main_utf16(std::unique_ptr<v8::String::Value> str) {
Expand Down Expand Up @@ -1321,6 +1353,10 @@ void Environment::set_process_exit_handler(
}
} // namespace node

// These two files depend on each other. Including base_object-inl.h after this
// file is the easiest way to avoid issues with that circular dependency.
#include "base_object-inl.h"

#endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS

#endif // SRC_ENV_INL_H_
36 changes: 26 additions & 10 deletions src/env.cc
Expand Up @@ -258,18 +258,30 @@ void TrackingTraceStateObserver::UpdateTraceCategoryState() {
USE(cb->Call(env_->context(), Undefined(isolate), arraysize(args), args));
}

class NoBindingData : public BaseObject {
public:
NoBindingData(Environment* env, Local<Object> obj) : BaseObject(env, obj) {}

SET_NO_MEMORY_INFO()
SET_MEMORY_INFO_NAME(NoBindingData)
SET_SELF_SIZE(NoBindingData)
};

void Environment::CreateProperties() {
HandleScope handle_scope(isolate_);
Local<Context> ctx = context();
Local<FunctionTemplate> templ = FunctionTemplate::New(isolate());
templ->InstanceTemplate()->SetInternalFieldCount(1);
Local<Object> obj = templ->GetFunction(ctx)
.ToLocalChecked()
->NewInstance(ctx)
.ToLocalChecked();
obj->SetAlignedPointerInInternalField(0, this);
set_as_callback_data(obj);
set_as_callback_data_template(templ);
{
Context::Scope context_scope(ctx);
Local<FunctionTemplate> templ = FunctionTemplate::New(isolate());
templ->InstanceTemplate()->SetInternalFieldCount(
BaseObject::kInternalFieldCount);
set_as_callback_data_template(templ);

Local<Object> obj = MakeBindingCallbackData<NoBindingData>()
.ToLocalChecked();
set_as_callback_data(obj);
set_current_callback_data(obj);
}

// Store primordials setup by the per-context script in the environment.
Local<Object> per_context_bindings =
Expand Down Expand Up @@ -417,6 +429,10 @@ Environment::Environment(IsolateData* isolate_data,
// TODO(joyeecheung): deserialize when the snapshot covers the environment
// properties.
CreateProperties();

// This adjusts the return value of base_object_count() so that tests that
// check the count do not have to account for internally created BaseObjects.
initial_base_object_count_ = base_object_count();
}

Environment::~Environment() {
Expand Down Expand Up @@ -467,7 +483,7 @@ Environment::~Environment() {
}
}

CHECK_EQ(base_object_count(), 0);
CHECK_EQ(base_object_count_, 0);
}

void Environment::InitializeLibuv(bool start_profiler_idle_notifier) {
Expand Down
20 changes: 20 additions & 0 deletions src/env.h
Expand Up @@ -436,6 +436,7 @@ constexpr size_t kFsStatsBufferLength =
V(async_hooks_promise_resolve_function, v8::Function) \
V(buffer_prototype_object, v8::Object) \
V(crypto_key_object_constructor, v8::Function) \
V(current_callback_data, v8::Object) \
V(domain_callback, v8::Function) \
V(domexception_function, v8::Function) \
V(enhance_fatal_stack_after_inspector, v8::Function) \
Expand Down Expand Up @@ -871,6 +872,24 @@ class Environment : public MemoryRetainer {

static inline Environment* GetFromCallbackData(v8::Local<v8::Value> val);

// Methods created using SetMethod(), SetPrototypeMethod(), etc. inside
// this scope can access the created T* object using
// Unwrap<T>(args.Data()) later.
template <typename T>
struct BindingScope {
explicit inline BindingScope(Environment* env);
inline ~BindingScope();

T* data = nullptr;
Environment* env;

inline operator bool() const { return data != nullptr; }
inline bool operator !() const { return data == nullptr; }
};

template <typename T>
inline v8::MaybeLocal<v8::Object> MakeBindingCallbackData();

static uv_key_t thread_local_env;
static inline Environment* GetThreadLocalEnv();

Expand Down Expand Up @@ -1461,6 +1480,7 @@ class Environment : public MemoryRetainer {
bool started_cleanup_ = false;

int64_t base_object_count_ = 0;
int64_t initial_base_object_count_ = 0;
std::atomic_bool is_stopping_ { false };

std::function<void(Environment*, int)> process_exit_handler_ {
Expand Down
2 changes: 1 addition & 1 deletion src/fs_event_wrap.cc
Expand Up @@ -108,7 +108,7 @@ void FSEventWrap::Initialize(Local<Object> target,
Local<FunctionTemplate> get_initialized_templ =
FunctionTemplate::New(env->isolate(),
GetInitialized,
env->as_callback_data(),
env->current_callback_data(),
Signature::New(env->isolate(), t));

t->PrototypeTemplate()->SetAccessorProperty(
Expand Down
2 changes: 1 addition & 1 deletion src/node.cc
Expand Up @@ -331,7 +331,7 @@ MaybeLocal<Value> Environment::BootstrapNode() {

Local<String> env_string = FIXED_ONE_BYTE_STRING(isolate_, "env");
Local<Object> env_var_proxy;
if (!CreateEnvVarProxy(context(), isolate_, as_callback_data())
if (!CreateEnvVarProxy(context(), isolate_, current_callback_data())
.ToLocal(&env_var_proxy) ||
process_object()->Set(context(), env_string, env_var_proxy).IsNothing()) {
return MaybeLocal<Value>();
Expand Down
4 changes: 2 additions & 2 deletions src/node_crypto.cc
Expand Up @@ -505,7 +505,7 @@ void SecureContext::Initialize(Environment* env, Local<Object> target) {
Local<FunctionTemplate> ctx_getter_templ =
FunctionTemplate::New(env->isolate(),
CtxGetter,
env->as_callback_data(),
env->current_callback_data(),
Signature::New(env->isolate(), t));


Expand Down Expand Up @@ -5101,7 +5101,7 @@ void DiffieHellman::Initialize(Environment* env, Local<Object> target) {
Local<FunctionTemplate> verify_error_getter_templ =
FunctionTemplate::New(env->isolate(),
DiffieHellman::VerifyErrorGetter,
env->as_callback_data(),
env->current_callback_data(),
Signature::New(env->isolate(), t),
/* length */ 0,
ConstructorBehavior::kThrow,
Expand Down
4 changes: 2 additions & 2 deletions src/node_process_object.cc
Expand Up @@ -147,7 +147,7 @@ void PatchProcessObject(const FunctionCallbackInfo<Value>& args) {
FIXED_ONE_BYTE_STRING(isolate, "title"),
ProcessTitleGetter,
env->owns_process_state() ? ProcessTitleSetter : nullptr,
env->as_callback_data(),
env->current_callback_data(),
DEFAULT,
None,
SideEffectType::kHasNoSideEffect)
Expand Down Expand Up @@ -198,7 +198,7 @@ void PatchProcessObject(const FunctionCallbackInfo<Value>& args) {
FIXED_ONE_BYTE_STRING(isolate, "debugPort"),
DebugPortGetter,
env->owns_process_state() ? DebugPortSetter : nullptr,
env->as_callback_data())
env->current_callback_data())
.FromJust());
}

Expand Down
2 changes: 1 addition & 1 deletion src/stream_wrap.cc
Expand Up @@ -142,7 +142,7 @@ Local<FunctionTemplate> LibuvStreamWrap::GetConstructorTemplate(
Local<FunctionTemplate> get_write_queue_size =
FunctionTemplate::New(env->isolate(),
GetWriteQueueSize,
env->as_callback_data(),
env->current_callback_data(),
Signature::New(env->isolate(), tmpl));
tmpl->PrototypeTemplate()->SetAccessorProperty(
env->write_queue_size_string(),
Expand Down
2 changes: 1 addition & 1 deletion src/tls_wrap.cc
Expand Up @@ -1276,7 +1276,7 @@ void TLSWrap::Initialize(Local<Object> target,
Local<FunctionTemplate> get_write_queue_size =
FunctionTemplate::New(env->isolate(),
GetWriteQueueSize,
env->as_callback_data(),
env->current_callback_data(),
Signature::New(env->isolate(), t));
t->PrototypeTemplate()->SetAccessorProperty(
env->write_queue_size_string(),
Expand Down
2 changes: 1 addition & 1 deletion src/udp_wrap.cc
Expand Up @@ -145,7 +145,7 @@ void UDPWrap::Initialize(Local<Object> target,
Local<FunctionTemplate> get_fd_templ =
FunctionTemplate::New(env->isolate(),
UDPWrap::GetFD,
env->as_callback_data(),
env->current_callback_data(),
signature);

t->PrototypeTemplate()->SetAccessorProperty(env->fd_string(),
Expand Down

0 comments on commit 6576b9b

Please sign in to comment.