Skip to content

Commit 4d5318c

Browse files
jasnellMylesBorins
jasnell
authored and
MylesBorins
committed
src: improve handling of internal field counting
Change suggested by bnoordhuis. Improve handing of internal field counting by using enums. Helps protect against future possible breakage if field indexes are ever changed or added to. Signed-off-by: James M Snell <jasnell@gmail.com> PR-URL: #31960 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
1 parent 0fc45f8 commit 4d5318c

40 files changed

+151
-92
lines changed

‎src/async_wrap.cc

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,10 @@ void AsyncWrap::EmitAfter(Environment* env, double async_id) {
176176

177177
class PromiseWrap : public AsyncWrap {
178178
public:
179+
enum InternalFields {
180+
kIsChainedPromiseField = AsyncWrap::kInternalFieldCount,
181+
kInternalFieldCount
182+
};
179183
PromiseWrap(Environment* env, Local<Object> object, bool silent)
180184
: AsyncWrap(env, object, PROVIDER_PROMISE, kInvalidAsyncId, silent) {
181185
MakeWeak();
@@ -185,9 +189,6 @@ class PromiseWrap : public AsyncWrap {
185189
SET_MEMORY_INFO_NAME(PromiseWrap)
186190
SET_SELF_SIZE(PromiseWrap)
187191

188-
static constexpr int kIsChainedPromiseField = 1;
189-
static constexpr int kInternalFieldCount = 2;
190-
191192
static PromiseWrap* New(Environment* env,
192193
Local<Promise> promise,
193194
PromiseWrap* parent_wrap,
@@ -214,15 +215,16 @@ PromiseWrap* PromiseWrap::New(Environment* env,
214215
void PromiseWrap::getIsChainedPromise(Local<String> property,
215216
const PropertyCallbackInfo<Value>& info) {
216217
info.GetReturnValue().Set(
217-
info.Holder()->GetInternalField(kIsChainedPromiseField));
218+
info.Holder()->GetInternalField(PromiseWrap::kIsChainedPromiseField));
218219
}
219220

220221
static PromiseWrap* extractPromiseWrap(Local<Promise> promise) {
221-
Local<Value> resource_object_value = promise->GetInternalField(0);
222-
if (resource_object_value->IsObject()) {
223-
return Unwrap<PromiseWrap>(resource_object_value.As<Object>());
224-
}
225-
return nullptr;
222+
// This check is imperfect. If the internal field is set, it should
223+
// be an object. If it's not, we just ignore it. Ideally v8 would
224+
// have had GetInternalField returning a MaybeLocal but this works
225+
// for now.
226+
Local<Value> obj = promise->GetInternalField(0);
227+
return obj->IsObject() ? Unwrap<PromiseWrap>(obj.As<Object>()) : nullptr;
226228
}
227229

228230
static void PromiseHook(PromiseHookType type, Local<Promise> promise,
@@ -560,7 +562,7 @@ void AsyncWrap::Initialize(Local<Object> target,
560562
function_template->SetClassName(class_name);
561563
function_template->Inherit(AsyncWrap::GetConstructorTemplate(env));
562564
auto instance_template = function_template->InstanceTemplate();
563-
instance_template->SetInternalFieldCount(1);
565+
instance_template->SetInternalFieldCount(AsyncWrap::kInternalFieldCount);
564566
auto function =
565567
function_template->GetFunction(env->context()).ToLocalChecked();
566568
target->Set(env->context(), class_name, function).Check();

‎src/base_object-inl.h

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,9 @@ BaseObject::BaseObject(Environment* env, v8::Local<v8::Object> object)
4343
: persistent_handle_(env->isolate(), object), env_(env) {
4444
CHECK_EQ(false, object.IsEmpty());
4545
CHECK_GT(object->InternalFieldCount(), 0);
46-
object->SetAlignedPointerInInternalField(0, static_cast<void*>(this));
46+
object->SetAlignedPointerInInternalField(
47+
BaseObject::kSlot,
48+
static_cast<void*>(this));
4749
env->AddCleanupHook(DeleteMe, static_cast<void*>(this));
4850
env->modify_base_object_count(1);
4951
}
@@ -67,7 +69,7 @@ BaseObject::~BaseObject() {
6769

6870
{
6971
v8::HandleScope handle_scope(env()->isolate());
70-
object()->SetAlignedPointerInInternalField(0, nullptr);
72+
object()->SetAlignedPointerInInternalField(BaseObject::kSlot, nullptr);
7173
}
7274
}
7375

@@ -100,7 +102,8 @@ Environment* BaseObject::env() const {
100102

101103
BaseObject* BaseObject::FromJSObject(v8::Local<v8::Object> obj) {
102104
CHECK_GT(obj->InternalFieldCount(), 0);
103-
return static_cast<BaseObject*>(obj->GetAlignedPointerFromInternalField(0));
105+
return static_cast<BaseObject*>(
106+
obj->GetAlignedPointerFromInternalField(BaseObject::kSlot));
104107
}
105108

106109

@@ -148,11 +151,12 @@ BaseObject::MakeLazilyInitializedJSTemplate(Environment* env) {
148151
auto constructor = [](const v8::FunctionCallbackInfo<v8::Value>& args) {
149152
DCHECK(args.IsConstructCall());
150153
DCHECK_GT(args.This()->InternalFieldCount(), 0);
151-
args.This()->SetAlignedPointerInInternalField(0, nullptr);
154+
args.This()->SetAlignedPointerInInternalField(BaseObject::kSlot, nullptr);
152155
};
153156

154157
v8::Local<v8::FunctionTemplate> t = env->NewFunctionTemplate(constructor);
155-
t->InstanceTemplate()->SetInternalFieldCount(1);
158+
t->InstanceTemplate()->SetInternalFieldCount(
159+
BaseObject::kInternalFieldCount);
156160
return t;
157161
}
158162

‎src/base_object.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@ class BaseObjectPtrImpl;
3636

3737
class BaseObject : public MemoryRetainer {
3838
public:
39+
enum InternalFields { kSlot, kInternalFieldCount };
40+
3941
// Associates this object with `object`. It uses the 0th internal field for
4042
// that, and in particular aborts if there is no such field.
4143
inline BaseObject(Environment* env, v8::Local<v8::Object> object);

‎src/cares_wrap.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2223,7 +2223,8 @@ void Initialize(Local<Object> target,
22232223

22242224
Local<FunctionTemplate> channel_wrap =
22252225
env->NewFunctionTemplate(ChannelWrap::New);
2226-
channel_wrap->InstanceTemplate()->SetInternalFieldCount(1);
2226+
channel_wrap->InstanceTemplate()->SetInternalFieldCount(
2227+
ChannelWrap::kInternalFieldCount);
22272228
channel_wrap->Inherit(AsyncWrap::GetConstructorTemplate(env));
22282229

22292230
env->SetProtoMethod(channel_wrap, "queryAny", Query<QueryAnyWrap>);

‎src/fs_event_wrap.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,8 @@ void FSEventWrap::Initialize(Local<Object> target,
9797

9898
auto fsevent_string = FIXED_ONE_BYTE_STRING(env->isolate(), "FSEvent");
9999
Local<FunctionTemplate> t = env->NewFunctionTemplate(New);
100-
t->InstanceTemplate()->SetInternalFieldCount(1);
100+
t->InstanceTemplate()->SetInternalFieldCount(
101+
FSEventWrap::kInternalFieldCount);
101102
t->SetClassName(fsevent_string);
102103

103104
t->Inherit(AsyncWrap::GetConstructorTemplate(env));

‎src/heap_utils.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -341,7 +341,7 @@ BaseObjectPtr<AsyncWrap> CreateHeapSnapshotStream(
341341
Local<FunctionTemplate> os = FunctionTemplate::New(env->isolate());
342342
os->Inherit(AsyncWrap::GetConstructorTemplate(env));
343343
Local<ObjectTemplate> ost = os->InstanceTemplate();
344-
ost->SetInternalFieldCount(StreamBase::kStreamBaseFieldCount);
344+
ost->SetInternalFieldCount(StreamBase::kInternalFieldCount);
345345
os->SetClassName(
346346
FIXED_ONE_BYTE_STRING(env->isolate(), "HeapSnapshotStream"));
347347
StreamBase::AddMethods(env, os);

‎src/inspector_js_api.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,8 @@ class JSBindingsConnection : public AsyncWrap {
105105
Local<String> class_name = ConnectionType::GetClassName(env);
106106
Local<FunctionTemplate> tmpl =
107107
env->NewFunctionTemplate(JSBindingsConnection::New);
108-
tmpl->InstanceTemplate()->SetInternalFieldCount(1);
108+
tmpl->InstanceTemplate()->SetInternalFieldCount(
109+
JSBindingsConnection::kInternalFieldCount);
109110
tmpl->SetClassName(class_name);
110111
tmpl->Inherit(AsyncWrap::GetConstructorTemplate(env));
111112
env->SetProtoMethod(tmpl, "dispatch", JSBindingsConnection::Dispatch);

‎src/js_stream.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ void JSStream::Initialize(Local<Object> target,
204204
FIXED_ONE_BYTE_STRING(env->isolate(), "JSStream");
205205
t->SetClassName(jsStreamString);
206206
t->InstanceTemplate()
207-
->SetInternalFieldCount(StreamBase::kStreamBaseFieldCount);
207+
->SetInternalFieldCount(StreamBase::kInternalFieldCount);
208208
t->Inherit(AsyncWrap::GetConstructorTemplate(env));
209209

210210
env->SetProtoMethod(t, "finishWrite", Finish<WriteWrap>);

‎src/module_wrap.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1636,7 +1636,8 @@ void ModuleWrap::Initialize(Local<Object> target,
16361636

16371637
Local<FunctionTemplate> tpl = env->NewFunctionTemplate(New);
16381638
tpl->SetClassName(FIXED_ONE_BYTE_STRING(isolate, "ModuleWrap"));
1639-
tpl->InstanceTemplate()->SetInternalFieldCount(1);
1639+
tpl->InstanceTemplate()->SetInternalFieldCount(
1640+
ModuleWrap::kInternalFieldCount);
16401641

16411642
env->SetProtoMethod(tpl, "link", Link);
16421643
env->SetProtoMethod(tpl, "instantiate", Instantiate);

‎src/node_contextify.cc

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ MaybeLocal<Object> ContextifyContext::CreateDataWrapper(Environment* env) {
145145
return MaybeLocal<Object>();
146146
}
147147

148-
wrapper->SetAlignedPointerInInternalField(0, this);
148+
wrapper->SetAlignedPointerInInternalField(ContextifyContext::kSlot, this);
149149
return wrapper;
150150
}
151151

@@ -232,7 +232,8 @@ MaybeLocal<Context> ContextifyContext::CreateV8Context(
232232
void ContextifyContext::Init(Environment* env, Local<Object> target) {
233233
Local<FunctionTemplate> function_template =
234234
FunctionTemplate::New(env->isolate());
235-
function_template->InstanceTemplate()->SetInternalFieldCount(1);
235+
function_template->InstanceTemplate()->SetInternalFieldCount(
236+
ContextifyContext::kInternalFieldCount);
236237
env->set_script_data_constructor_function(
237238
function_template->GetFunction(env->context()).ToLocalChecked());
238239

@@ -331,7 +332,8 @@ template <typename T>
331332
ContextifyContext* ContextifyContext::Get(const PropertyCallbackInfo<T>& args) {
332333
Local<Value> data = args.Data();
333334
return static_cast<ContextifyContext*>(
334-
data.As<Object>()->GetAlignedPointerFromInternalField(0));
335+
data.As<Object>()->GetAlignedPointerFromInternalField(
336+
ContextifyContext::kSlot));
335337
}
336338

337339
// static
@@ -628,7 +630,8 @@ void ContextifyScript::Init(Environment* env, Local<Object> target) {
628630
FIXED_ONE_BYTE_STRING(env->isolate(), "ContextifyScript");
629631

630632
Local<FunctionTemplate> script_tmpl = env->NewFunctionTemplate(New);
631-
script_tmpl->InstanceTemplate()->SetInternalFieldCount(1);
633+
script_tmpl->InstanceTemplate()->SetInternalFieldCount(
634+
ContextifyScript::kInternalFieldCount);
632635
script_tmpl->SetClassName(class_name);
633636
env->SetProtoMethod(script_tmpl, "createCachedData", CreateCachedData);
634637
env->SetProtoMethod(script_tmpl, "runInContext", RunInContext);
@@ -1251,7 +1254,8 @@ void Initialize(Local<Object> target,
12511254
{
12521255
Local<FunctionTemplate> tpl = FunctionTemplate::New(env->isolate());
12531256
tpl->SetClassName(FIXED_ONE_BYTE_STRING(env->isolate(), "CompiledFnEntry"));
1254-
tpl->InstanceTemplate()->SetInternalFieldCount(1);
1257+
tpl->InstanceTemplate()->SetInternalFieldCount(
1258+
CompiledFnEntry::kInternalFieldCount);
12551259

12561260
env->set_compiled_fn_entry_template(tpl->InstanceTemplate());
12571261
}

0 commit comments

Comments
 (0)