Skip to content

Commit 9c6e9cd

Browse files
jasnelljasnell
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>
1 parent 24aa9bd commit 9c6e9cd

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
@@ -1639,7 +1639,8 @@ void ModuleWrap::Initialize(Local<Object> target,
16391639

16401640
Local<FunctionTemplate> tpl = env->NewFunctionTemplate(New);
16411641
tpl->SetClassName(FIXED_ONE_BYTE_STRING(isolate, "ModuleWrap"));
1642-
tpl->InstanceTemplate()->SetInternalFieldCount(1);
1642+
tpl->InstanceTemplate()->SetInternalFieldCount(
1643+
ModuleWrap::kInternalFieldCount);
16431644

16441645
env->SetProtoMethod(tpl, "link", Link);
16451646
env->SetProtoMethod(tpl, "instantiate", Instantiate);

‎src/node_contextify.cc

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

145-
wrapper->SetAlignedPointerInInternalField(0, this);
145+
wrapper->SetAlignedPointerInInternalField(ContextifyContext::kSlot, this);
146146
return wrapper;
147147
}
148148

@@ -229,7 +229,8 @@ MaybeLocal<Context> ContextifyContext::CreateV8Context(
229229
void ContextifyContext::Init(Environment* env, Local<Object> target) {
230230
Local<FunctionTemplate> function_template =
231231
FunctionTemplate::New(env->isolate());
232-
function_template->InstanceTemplate()->SetInternalFieldCount(1);
232+
function_template->InstanceTemplate()->SetInternalFieldCount(
233+
ContextifyContext::kInternalFieldCount);
233234
env->set_script_data_constructor_function(
234235
function_template->GetFunction(env->context()).ToLocalChecked());
235236

@@ -328,7 +329,8 @@ template <typename T>
328329
ContextifyContext* ContextifyContext::Get(const PropertyCallbackInfo<T>& args) {
329330
Local<Value> data = args.Data();
330331
return static_cast<ContextifyContext*>(
331-
data.As<Object>()->GetAlignedPointerFromInternalField(0));
332+
data.As<Object>()->GetAlignedPointerFromInternalField(
333+
ContextifyContext::kSlot));
332334
}
333335

334336
// static
@@ -625,7 +627,8 @@ void ContextifyScript::Init(Environment* env, Local<Object> target) {
625627
FIXED_ONE_BYTE_STRING(env->isolate(), "ContextifyScript");
626628

627629
Local<FunctionTemplate> script_tmpl = env->NewFunctionTemplate(New);
628-
script_tmpl->InstanceTemplate()->SetInternalFieldCount(1);
630+
script_tmpl->InstanceTemplate()->SetInternalFieldCount(
631+
ContextifyScript::kInternalFieldCount);
629632
script_tmpl->SetClassName(class_name);
630633
env->SetProtoMethod(script_tmpl, "createCachedData", CreateCachedData);
631634
env->SetProtoMethod(script_tmpl, "runInContext", RunInContext);
@@ -1220,7 +1223,8 @@ void Initialize(Local<Object> target,
12201223
{
12211224
Local<FunctionTemplate> tpl = FunctionTemplate::New(env->isolate());
12221225
tpl->SetClassName(FIXED_ONE_BYTE_STRING(env->isolate(), "CompiledFnEntry"));
1223-
tpl->InstanceTemplate()->SetInternalFieldCount(1);
1226+
tpl->InstanceTemplate()->SetInternalFieldCount(
1227+
CompiledFnEntry::kInternalFieldCount);
12241228

12251229
env->set_compiled_fn_entry_template(tpl->InstanceTemplate());
12261230
}

‎src/node_contextify.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ struct ContextOptions {
1919

2020
class ContextifyContext {
2121
public:
22+
enum InternalFields { kSlot, kInternalFieldCount };
2223
ContextifyContext(Environment* env,
2324
v8::Local<v8::Object> sandbox_obj,
2425
const ContextOptions& options);

‎src/node_crypto.cc

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -464,7 +464,8 @@ static T* MallocOpenSSL(size_t count) {
464464

465465
void SecureContext::Initialize(Environment* env, Local<Object> target) {
466466
Local<FunctionTemplate> t = env->NewFunctionTemplate(New);
467-
t->InstanceTemplate()->SetInternalFieldCount(1);
467+
t->InstanceTemplate()->SetInternalFieldCount(
468+
SecureContext::kInternalFieldCount);
468469
Local<String> secureContextString =
469470
FIXED_ONE_BYTE_STRING(env->isolate(), "SecureContext");
470471
t->SetClassName(secureContextString);
@@ -3807,7 +3808,8 @@ EVP_PKEY* ManagedEVPPKey::get() const {
38073808

38083809
Local<Function> KeyObject::Initialize(Environment* env, Local<Object> target) {
38093810
Local<FunctionTemplate> t = env->NewFunctionTemplate(New);
3810-
t->InstanceTemplate()->SetInternalFieldCount(1);
3811+
t->InstanceTemplate()->SetInternalFieldCount(
3812+
KeyObject::kInternalFieldCount);
38113813

38123814
env->SetProtoMethod(t, "init", Init);
38133815
env->SetProtoMethodNoSideEffect(t, "getSymmetricKeySize",
@@ -4040,7 +4042,8 @@ CipherBase::CipherBase(Environment* env,
40404042
void CipherBase::Initialize(Environment* env, Local<Object> target) {
40414043
Local<FunctionTemplate> t = env->NewFunctionTemplate(New);
40424044

4043-
t->InstanceTemplate()->SetInternalFieldCount(1);
4045+
t->InstanceTemplate()->SetInternalFieldCount(
4046+
CipherBase::kInternalFieldCount);
40444047

40454048
env->SetProtoMethod(t, "init", Init);
40464049
env->SetProtoMethod(t, "initiv", InitIv);
@@ -4667,7 +4670,8 @@ Hmac::Hmac(Environment* env, v8::Local<v8::Object> wrap)
46674670
void Hmac::Initialize(Environment* env, Local<Object> target) {
46684671
Local<FunctionTemplate> t = env->NewFunctionTemplate(New);
46694672

4670-
t->InstanceTemplate()->SetInternalFieldCount(1);
4673+
t->InstanceTemplate()->SetInternalFieldCount(
4674+
Hmac::kInternalFieldCount);
46714675

46724676
env->SetProtoMethod(t, "init", HmacInit);
46734677
env->SetProtoMethod(t, "update", HmacUpdate);
@@ -4793,7 +4797,8 @@ Hash::Hash(Environment* env, v8::Local<v8::Object> wrap)
47934797
void Hash::Initialize(Environment* env, Local<Object> target) {
47944798
Local<FunctionTemplate> t = env->NewFunctionTemplate(New);
47954799

4796-
t->InstanceTemplate()->SetInternalFieldCount(1);
4800+
t->InstanceTemplate()->SetInternalFieldCount(
4801+
Hash::kInternalFieldCount);
47974802

47984803
env->SetProtoMethod(t, "update", HashUpdate);
47994804
env->SetProtoMethod(t, "digest", HashDigest);
@@ -5065,7 +5070,8 @@ Sign::Sign(Environment* env, v8::Local<v8::Object> wrap) : SignBase(env, wrap) {
50655070
void Sign::Initialize(Environment* env, Local<Object> target) {
50665071
Local<FunctionTemplate> t = env->NewFunctionTemplate(New);
50675072

5068-
t->InstanceTemplate()->SetInternalFieldCount(1);
5073+
t->InstanceTemplate()->SetInternalFieldCount(
5074+
SignBase::kInternalFieldCount);
50695075

50705076
env->SetProtoMethod(t, "init", SignInit);
50715077
env->SetProtoMethod(t, "update", SignUpdate);
@@ -5389,7 +5395,8 @@ Verify::Verify(Environment* env, v8::Local<v8::Object> wrap) :
53895395
void Verify::Initialize(Environment* env, Local<Object> target) {
53905396
Local<FunctionTemplate> t = env->NewFunctionTemplate(New);
53915397

5392-
t->InstanceTemplate()->SetInternalFieldCount(1);
5398+
t->InstanceTemplate()->SetInternalFieldCount(
5399+
SignBase::kInternalFieldCount);
53935400

53945401
env->SetProtoMethod(t, "init", VerifyInit);
53955402
env->SetProtoMethod(t, "update", VerifyUpdate);
@@ -5701,7 +5708,8 @@ void DiffieHellman::Initialize(Environment* env, Local<Object> target) {
57015708
const PropertyAttribute attributes =
57025709
static_cast<PropertyAttribute>(ReadOnly | DontDelete);
57035710

5704-
t->InstanceTemplate()->SetInternalFieldCount(1);
5711+
t->InstanceTemplate()->SetInternalFieldCount(
5712+
DiffieHellman::kInternalFieldCount);
57055713

57065714
env->SetProtoMethod(t, "generateKeys", GenerateKeys);
57075715
env->SetProtoMethod(t, "computeSecret", ComputeSecret);
@@ -6048,7 +6056,7 @@ void ECDH::Initialize(Environment* env, Local<Object> target) {
60486056

60496057
Local<FunctionTemplate> t = env->NewFunctionTemplate(New);
60506058

6051-
t->InstanceTemplate()->SetInternalFieldCount(1);
6059+
t->InstanceTemplate()->SetInternalFieldCount(ECDH::kInternalFieldCount);
60526060

60536061
env->SetProtoMethod(t, "generateKeys", GenerateKeys);
60546062
env->SetProtoMethod(t, "computeSecret", ComputeSecret);

‎src/node_dir.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -358,7 +358,7 @@ void Initialize(Local<Object> target,
358358
env->SetProtoMethod(dir, "read", DirHandle::Read);
359359
env->SetProtoMethod(dir, "close", DirHandle::Close);
360360
Local<ObjectTemplate> dirt = dir->InstanceTemplate();
361-
dirt->SetInternalFieldCount(DirHandle::kDirHandleFieldCount);
361+
dirt->SetInternalFieldCount(DirHandle::kInternalFieldCount);
362362
Local<String> handleString =
363363
FIXED_ONE_BYTE_STRING(isolate, "DirHandle");
364364
dir->SetClassName(handleString);

‎src/node_dir.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,6 @@ namespace fs_dir {
1212
// Needed to propagate `uv_dir_t`.
1313
class DirHandle : public AsyncWrap {
1414
public:
15-
static constexpr int kDirHandleFieldCount = 1;
16-
1715
static DirHandle* New(Environment* env, uv_dir_t* dir);
1816
~DirHandle() override;
1917

‎src/node_file.cc

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2290,7 +2290,8 @@ void Initialize(Local<Object> target,
22902290

22912291
// Create FunctionTemplate for FSReqCallback
22922292
Local<FunctionTemplate> fst = env->NewFunctionTemplate(NewFSReqCallback);
2293-
fst->InstanceTemplate()->SetInternalFieldCount(1);
2293+
fst->InstanceTemplate()->SetInternalFieldCount(
2294+
FSReqBase::kInternalFieldCount);
22942295
fst->Inherit(AsyncWrap::GetConstructorTemplate(env));
22952296
Local<String> wrapString =
22962297
FIXED_ONE_BYTE_STRING(isolate, "FSReqCallback");
@@ -2303,7 +2304,8 @@ void Initialize(Local<Object> target,
23032304
// Create FunctionTemplate for FileHandleReadWrap. There’s no need
23042305
// to do anything in the constructor, so we only store the instance template.
23052306
Local<FunctionTemplate> fh_rw = FunctionTemplate::New(isolate);
2306-
fh_rw->InstanceTemplate()->SetInternalFieldCount(1);
2307+
fh_rw->InstanceTemplate()->SetInternalFieldCount(
2308+
FSReqBase::kInternalFieldCount);
23072309
fh_rw->Inherit(AsyncWrap::GetConstructorTemplate(env));
23082310
Local<String> fhWrapString =
23092311
FIXED_ONE_BYTE_STRING(isolate, "FileHandleReqWrap");
@@ -2318,7 +2320,7 @@ void Initialize(Local<Object> target,
23182320
FIXED_ONE_BYTE_STRING(isolate, "FSReqPromise");
23192321
fpt->SetClassName(promiseString);
23202322
Local<ObjectTemplate> fpo = fpt->InstanceTemplate();
2321-
fpo->SetInternalFieldCount(1);
2323+
fpo->SetInternalFieldCount(FSReqBase::kInternalFieldCount);
23222324
env->set_fsreqpromise_constructor_template(fpo);
23232325

23242326
// Create FunctionTemplate for FileHandle
@@ -2327,7 +2329,7 @@ void Initialize(Local<Object> target,
23272329
env->SetProtoMethod(fd, "close", FileHandle::Close);
23282330
env->SetProtoMethod(fd, "releaseFD", FileHandle::ReleaseFD);
23292331
Local<ObjectTemplate> fdt = fd->InstanceTemplate();
2330-
fdt->SetInternalFieldCount(StreamBase::kStreamBaseFieldCount);
2332+
fdt->SetInternalFieldCount(StreamBase::kInternalFieldCount);
23312333
Local<String> handleString =
23322334
FIXED_ONE_BYTE_STRING(isolate, "FileHandle");
23332335
fd->SetClassName(handleString);
@@ -2344,7 +2346,7 @@ void Initialize(Local<Object> target,
23442346
"FileHandleCloseReq"));
23452347
fdclose->Inherit(AsyncWrap::GetConstructorTemplate(env));
23462348
Local<ObjectTemplate> fdcloset = fdclose->InstanceTemplate();
2347-
fdcloset->SetInternalFieldCount(1);
2349+
fdcloset->SetInternalFieldCount(FSReqBase::kInternalFieldCount);
23482350
env->set_fdclose_constructor_template(fdcloset);
23492351

23502352
Local<Symbol> use_promises_symbol =

0 commit comments

Comments
 (0)