Skip to content

Commit 1fc3de9

Browse files
addaleaxBethGriggs
addaleax
authored and
BethGriggs
committed
src: make creating per-binding data structures easier
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 <[email protected]>
1 parent dbd030a commit 1fc3de9

12 files changed

Lines changed: 104 additions & 31 deletions

‎src/base_object-inl.h‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -100,15 +100,16 @@ Environment* BaseObject::env() const {
100100
return env_;
101101
}
102102

103-
BaseObject* BaseObject::FromJSObject(v8::Local<v8::Object> obj) {
104-
CHECK_GT(obj->InternalFieldCount(), 0);
103+
BaseObject* BaseObject::FromJSObject(v8::Local<v8::Value> value) {
104+
v8::Local<v8::Object> obj = value.As<v8::Object>();
105+
DCHECK_GE(obj->InternalFieldCount(), BaseObject::kSlot);
105106
return static_cast<BaseObject*>(
106107
obj->GetAlignedPointerFromInternalField(BaseObject::kSlot));
107108
}
108109

109110

110111
template <typename T>
111-
T* BaseObject::FromJSObject(v8::Local<v8::Object> object) {
112+
T* BaseObject::FromJSObject(v8::Local<v8::Value> object) {
112113
return static_cast<T*>(FromJSObject(object));
113114
}
114115

‎src/base_object.h‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,9 @@ class BaseObject : public MemoryRetainer {
6161
// was also passed to the `BaseObject()` constructor initially.
6262
// This may return `nullptr` if the C++ object has not been constructed yet,
6363
// e.g. when the JS object used `MakeLazilyInitializedJSTemplate`.
64-
static inline BaseObject* FromJSObject(v8::Local<v8::Object> object);
64+
static inline BaseObject* FromJSObject(v8::Local<v8::Value> object);
6565
template <typename T>
66-
static inline T* FromJSObject(v8::Local<v8::Object> object);
66+
static inline T* FromJSObject(v8::Local<v8::Value> object);
6767

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

153153
// Global alias for FromJSObject() to avoid churn.
154154
template <typename T>
155-
inline T* Unwrap(v8::Local<v8::Object> obj) {
155+
inline T* Unwrap(v8::Local<v8::Value> obj) {
156156
return BaseObject::FromJSObject<T>(obj);
157157
}
158158

‎src/env-inl.h‎

Lines changed: 42 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -327,16 +327,48 @@ inline Environment* Environment::GetCurrent(
327327
return GetFromCallbackData(info.Data());
328328
}
329329

330-
inline Environment* Environment::GetFromCallbackData(v8::Local<v8::Value> val) {
330+
Environment* Environment::GetFromCallbackData(v8::Local<v8::Value> val) {
331331
DCHECK(val->IsObject());
332332
v8::Local<v8::Object> obj = val.As<v8::Object>();
333-
DCHECK_GE(obj->InternalFieldCount(), 1);
334-
Environment* env =
335-
static_cast<Environment*>(obj->GetAlignedPointerFromInternalField(0));
333+
DCHECK_GE(obj->InternalFieldCount(),
334+
BaseObject::kInternalFieldCount);
335+
Environment* env = Unwrap<BaseObject>(obj)->env();
336336
DCHECK(env->as_callback_data_template()->HasInstance(obj));
337337
return env;
338338
}
339339

340+
template <typename T>
341+
Environment::BindingScope<T>::BindingScope(Environment* env) : env(env) {
342+
v8::Local<v8::Object> callback_data;
343+
if (!env->MakeBindingCallbackData<T>().ToLocal(&callback_data))
344+
return;
345+
data = Unwrap<T>(callback_data);
346+
347+
// No nesting allowed currently.
348+
CHECK_EQ(env->current_callback_data(), env->as_callback_data());
349+
env->set_current_callback_data(callback_data);
350+
}
351+
352+
template <typename T>
353+
Environment::BindingScope<T>::~BindingScope() {
354+
env->set_current_callback_data(env->as_callback_data());
355+
}
356+
357+
template <typename T>
358+
v8::MaybeLocal<v8::Object> Environment::MakeBindingCallbackData() {
359+
v8::Local<v8::Function> ctor;
360+
v8::Local<v8::Object> obj;
361+
if (!as_callback_data_template()->GetFunction(context()).ToLocal(&ctor) ||
362+
!ctor->NewInstance(context()).ToLocal(&obj)) {
363+
return v8::MaybeLocal<v8::Object>();
364+
}
365+
T* data = new T(this, obj);
366+
// This won't compile if T is not a BaseObject subclass.
367+
CHECK_EQ(data, static_cast<BaseObject*>(data));
368+
data->MakeWeak();
369+
return obj;
370+
}
371+
340372
inline Environment* Environment::GetThreadLocalEnv() {
341373
return static_cast<Environment*>(uv_key_get(&thread_local_env));
342374
}
@@ -1115,7 +1147,7 @@ inline v8::Local<v8::FunctionTemplate>
11151147
v8::Local<v8::Signature> signature,
11161148
v8::ConstructorBehavior behavior,
11171149
v8::SideEffectType side_effect_type) {
1118-
v8::Local<v8::Object> external = as_callback_data();
1150+
v8::Local<v8::Object> external = current_callback_data();
11191151
return v8::FunctionTemplate::New(isolate(), callback, external,
11201152
signature, 0, behavior, side_effect_type);
11211153
}
@@ -1253,7 +1285,7 @@ void Environment::modify_base_object_count(int64_t delta) {
12531285
}
12541286

12551287
int64_t Environment::base_object_count() const {
1256-
return base_object_count_;
1288+
return base_object_count_ - initial_base_object_count_;
12571289
}
12581290

12591291
void Environment::set_main_utf16(std::unique_ptr<v8::String::Value> str) {
@@ -1313,6 +1345,10 @@ void Environment::set_process_exit_handler(
13131345
}
13141346
} // namespace node
13151347

1348+
// These two files depend on each other. Including base_object-inl.h after this
1349+
// file is the easiest way to avoid issues with that circular dependency.
1350+
#include "base_object-inl.h"
1351+
13161352
#endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
13171353

13181354
#endif // SRC_ENV_INL_H_

‎src/env.cc‎

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -258,18 +258,30 @@ void TrackingTraceStateObserver::UpdateTraceCategoryState() {
258258
USE(cb->Call(env_->context(), Undefined(isolate), arraysize(args), args));
259259
}
260260

261+
class NoBindingData : public BaseObject {
262+
public:
263+
NoBindingData(Environment* env, Local<Object> obj) : BaseObject(env, obj) {}
264+
265+
SET_NO_MEMORY_INFO()
266+
SET_MEMORY_INFO_NAME(NoBindingData)
267+
SET_SELF_SIZE(NoBindingData)
268+
};
269+
261270
void Environment::CreateProperties() {
262271
HandleScope handle_scope(isolate_);
263272
Local<Context> ctx = context();
264-
Local<FunctionTemplate> templ = FunctionTemplate::New(isolate());
265-
templ->InstanceTemplate()->SetInternalFieldCount(1);
266-
Local<Object> obj = templ->GetFunction(ctx)
267-
.ToLocalChecked()
268-
->NewInstance(ctx)
269-
.ToLocalChecked();
270-
obj->SetAlignedPointerInInternalField(0, this);
271-
set_as_callback_data(obj);
272-
set_as_callback_data_template(templ);
273+
{
274+
Context::Scope context_scope(ctx);
275+
Local<FunctionTemplate> templ = FunctionTemplate::New(isolate());
276+
templ->InstanceTemplate()->SetInternalFieldCount(
277+
BaseObject::kInternalFieldCount);
278+
set_as_callback_data_template(templ);
279+
280+
Local<Object> obj = MakeBindingCallbackData<NoBindingData>()
281+
.ToLocalChecked();
282+
set_as_callback_data(obj);
283+
set_current_callback_data(obj);
284+
}
273285

274286
// Store primordials setup by the per-context script in the environment.
275287
Local<Object> per_context_bindings =
@@ -417,6 +429,10 @@ Environment::Environment(IsolateData* isolate_data,
417429
// TODO(joyeecheung): deserialize when the snapshot covers the environment
418430
// properties.
419431
CreateProperties();
432+
433+
// This adjusts the return value of base_object_count() so that tests that
434+
// check the count do not have to account for internally created BaseObjects.
435+
initial_base_object_count_ = base_object_count();
420436
}
421437

422438
Environment::~Environment() {
@@ -467,7 +483,7 @@ Environment::~Environment() {
467483
}
468484
}
469485

470-
CHECK_EQ(base_object_count(), 0);
486+
CHECK_EQ(base_object_count_, 0);
471487
}
472488

473489
void Environment::InitializeLibuv(bool start_profiler_idle_notifier) {

‎src/env.h‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -436,6 +436,7 @@ constexpr size_t kFsStatsBufferLength =
436436
V(async_hooks_promise_resolve_function, v8::Function) \
437437
V(buffer_prototype_object, v8::Object) \
438438
V(crypto_key_object_constructor, v8::Function) \
439+
V(current_callback_data, v8::Object) \
439440
V(domain_callback, v8::Function) \
440441
V(domexception_function, v8::Function) \
441442
V(enhance_fatal_stack_after_inspector, v8::Function) \
@@ -871,6 +872,24 @@ class Environment : public MemoryRetainer {
871872

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

875+
// Methods created using SetMethod(), SetPrototypeMethod(), etc. inside
876+
// this scope can access the created T* object using
877+
// Unwrap<T>(args.Data()) later.
878+
template <typename T>
879+
struct BindingScope {
880+
explicit inline BindingScope(Environment* env);
881+
inline ~BindingScope();
882+
883+
T* data = nullptr;
884+
Environment* env;
885+
886+
inline operator bool() const { return data != nullptr; }
887+
inline bool operator !() const { return data == nullptr; }
888+
};
889+
890+
template <typename T>
891+
inline v8::MaybeLocal<v8::Object> MakeBindingCallbackData();
892+
874893
static uv_key_t thread_local_env;
875894
static inline Environment* GetThreadLocalEnv();
876895

@@ -1458,6 +1477,7 @@ class Environment : public MemoryRetainer {
14581477
bool started_cleanup_ = false;
14591478

14601479
int64_t base_object_count_ = 0;
1480+
int64_t initial_base_object_count_ = 0;
14611481
std::atomic_bool is_stopping_ { false };
14621482

14631483
std::function<void(Environment*, int)> process_exit_handler_ {

‎src/fs_event_wrap.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ void FSEventWrap::Initialize(Local<Object> target,
108108
Local<FunctionTemplate> get_initialized_templ =
109109
FunctionTemplate::New(env->isolate(),
110110
GetInitialized,
111-
env->as_callback_data(),
111+
env->current_callback_data(),
112112
Signature::New(env->isolate(), t));
113113

114114
t->PrototypeTemplate()->SetAccessorProperty(

‎src/node.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -331,7 +331,7 @@ MaybeLocal<Value> Environment::BootstrapNode() {
331331

332332
Local<String> env_string = FIXED_ONE_BYTE_STRING(isolate_, "env");
333333
Local<Object> env_var_proxy;
334-
if (!CreateEnvVarProxy(context(), isolate_, as_callback_data())
334+
if (!CreateEnvVarProxy(context(), isolate_, current_callback_data())
335335
.ToLocal(&env_var_proxy) ||
336336
process_object()->Set(context(), env_string, env_var_proxy).IsNothing()) {
337337
return MaybeLocal<Value>();

‎src/node_crypto.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -505,7 +505,7 @@ void SecureContext::Initialize(Environment* env, Local<Object> target) {
505505
Local<FunctionTemplate> ctx_getter_templ =
506506
FunctionTemplate::New(env->isolate(),
507507
CtxGetter,
508-
env->as_callback_data(),
508+
env->current_callback_data(),
509509
Signature::New(env->isolate(), t));
510510

511511

@@ -5101,7 +5101,7 @@ void DiffieHellman::Initialize(Environment* env, Local<Object> target) {
51015101
Local<FunctionTemplate> verify_error_getter_templ =
51025102
FunctionTemplate::New(env->isolate(),
51035103
DiffieHellman::VerifyErrorGetter,
5104-
env->as_callback_data(),
5104+
env->current_callback_data(),
51055105
Signature::New(env->isolate(), t),
51065106
/* length */ 0,
51075107
ConstructorBehavior::kThrow,

‎src/node_process_object.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ void PatchProcessObject(const FunctionCallbackInfo<Value>& args) {
147147
FIXED_ONE_BYTE_STRING(isolate, "title"),
148148
ProcessTitleGetter,
149149
env->owns_process_state() ? ProcessTitleSetter : nullptr,
150-
env->as_callback_data(),
150+
env->current_callback_data(),
151151
DEFAULT,
152152
None,
153153
SideEffectType::kHasNoSideEffect)
@@ -198,7 +198,7 @@ void PatchProcessObject(const FunctionCallbackInfo<Value>& args) {
198198
FIXED_ONE_BYTE_STRING(isolate, "debugPort"),
199199
DebugPortGetter,
200200
env->owns_process_state() ? DebugPortSetter : nullptr,
201-
env->as_callback_data())
201+
env->current_callback_data())
202202
.FromJust());
203203
}
204204

‎src/stream_wrap.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ Local<FunctionTemplate> LibuvStreamWrap::GetConstructorTemplate(
142142
Local<FunctionTemplate> get_write_queue_size =
143143
FunctionTemplate::New(env->isolate(),
144144
GetWriteQueueSize,
145-
env->as_callback_data(),
145+
env->current_callback_data(),
146146
Signature::New(env->isolate(), tmpl));
147147
tmpl->PrototypeTemplate()->SetAccessorProperty(
148148
env->write_queue_size_string(),

0 commit comments

Comments
 (0)