Skip to content

Commit caa7858

Browse files
codebytereaduh95
codebytere
authored and
aduh95
committed
src: fix abort when two Environments share an IsolateData
A second Environment created from the same `IsolateData`, which embedding.md allows and the cctest fixture does, aborted as soon as it loaded `stream_wrap`, `tcp_wrap`, `pipe_wrap`, `tty_wrap`, `http2` or the crypto key classes, i.e. on `require('net')`: those bindings created their per-isolate templates from the per-Environment binding initializer and stored them with a setter that CHECKs the slot is empty. Templates that were already cached per isolate (ffi, sqlite, dtls) instead hit "FunctionTemplate already instantiated" when `SetConstructorFunction()` renamed them. Build these templates once per isolate and reuse them, set class names when the template is created, and store the `ShutdownWrap`, `WriteWrap` and `Http2Stream` function templates rather than their instance templates so the second Environment can expose the same constructors. Refs: #43802 Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com> PR-URL: #65978 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Tim Perry <pimterry@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>
1 parent 886fd32 commit caa7858

14 files changed

Lines changed: 226 additions & 152 deletions

‎src/crypto/crypto_keys.cc‎

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1692,12 +1692,13 @@ void NativeKeyObject::CreateNativeKeyObjectClass(
16921692
Local<Value> callback = args[0];
16931693
CHECK(callback->IsFunction());
16941694

1695-
Local<FunctionTemplate> t =
1696-
NewFunctionTemplate(isolate, NativeKeyObject::New);
1697-
t->InstanceTemplate()->SetInternalFieldCount(
1698-
NativeKeyObject::kInternalFieldCount);
1699-
CHECK(env->crypto_key_object_constructor_template().IsEmpty());
1700-
env->set_crypto_key_object_constructor_template(t);
1695+
Local<FunctionTemplate> t = env->crypto_key_object_constructor_template();
1696+
if (t.IsEmpty()) {
1697+
t = NewFunctionTemplate(isolate, NativeKeyObject::New);
1698+
t->InstanceTemplate()->SetInternalFieldCount(
1699+
NativeKeyObject::kInternalFieldCount);
1700+
env->set_crypto_key_object_constructor_template(t);
1701+
}
17011702

17021703
Local<Value> ctor;
17031704
if (!t->GetFunction(env->context()).ToLocal(&ctor))
@@ -1929,12 +1930,13 @@ void NativeCryptoKey::CreateCryptoKeyClass(
19291930
Local<Value> callback = args[0];
19301931
CHECK(callback->IsFunction());
19311932

1932-
Local<FunctionTemplate> t =
1933-
NewFunctionTemplate(isolate, NativeCryptoKey::New);
1934-
t->InstanceTemplate()->SetInternalFieldCount(
1935-
NativeCryptoKey::kInternalFieldCount);
1936-
CHECK(env->crypto_cryptokey_constructor_template().IsEmpty());
1937-
env->set_crypto_cryptokey_constructor_template(t);
1933+
Local<FunctionTemplate> t = env->crypto_cryptokey_constructor_template();
1934+
if (t.IsEmpty()) {
1935+
t = NewFunctionTemplate(isolate, NativeCryptoKey::New);
1936+
t->InstanceTemplate()->SetInternalFieldCount(
1937+
NativeCryptoKey::kInternalFieldCount);
1938+
env->set_crypto_cryptokey_constructor_template(t);
1939+
}
19381940

19391941
Local<Value> ctor;
19401942
if (!t->GetFunction(env->context()).ToLocal(&ctor)) return;

‎src/dtls/dtls_context.cc‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -170,8 +170,11 @@ Local<FunctionTemplate> DTLSContext::GetConstructorTemplate(Environment* env) {
170170
void DTLSContext::InitPerContext(Local<Object> target,
171171
Local<Context> context,
172172
Environment* env) {
173-
SetConstructorFunction(
174-
context, target, "DTLSContext", GetConstructorTemplate(env));
173+
SetConstructorFunction(context,
174+
target,
175+
"DTLSContext",
176+
GetConstructorTemplate(env),
177+
SetConstructorFunctionFlag::NONE);
175178
}
176179

177180
void DTLSContext::RegisterExternalReferences(

‎src/dtls/dtls_endpoint.cc‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,8 +108,11 @@ Local<FunctionTemplate> DTLSEndpoint::GetConstructorTemplate(Environment* env) {
108108
void DTLSEndpoint::InitPerContext(Local<Object> target,
109109
Local<Context> context,
110110
Environment* env) {
111-
SetConstructorFunction(
112-
context, target, "DTLSEndpoint", GetConstructorTemplate(env));
111+
SetConstructorFunction(context,
112+
target,
113+
"DTLSEndpoint",
114+
GetConstructorTemplate(env),
115+
SetConstructorFunctionFlag::NONE);
113116
}
114117

115118
void DTLSEndpoint::RegisterExternalReferences(

‎src/dtls/dtls_session.cc‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -204,8 +204,11 @@ Local<FunctionTemplate> DTLSSession::GetConstructorTemplate(Environment* env) {
204204
void DTLSSession::InitPerContext(Local<Object> target,
205205
Local<Context> context,
206206
Environment* env) {
207-
SetConstructorFunction(
208-
context, target, "DTLSSession", GetConstructorTemplate(env));
207+
SetConstructorFunction(context,
208+
target,
209+
"DTLSSession",
210+
GetConstructorTemplate(env),
211+
SetConstructorFunctionFlag::NONE);
209212
}
210213

211214
void DTLSSession::RegisterExternalReferences(

‎src/env_properties.h‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -424,7 +424,7 @@
424424
V(v8_heap_statistics_template, v8::DictionaryTemplate) \
425425
V(histogram_ctor_template, v8::FunctionTemplate) \
426426
V(http2settings_constructor_template, v8::ObjectTemplate) \
427-
V(http2stream_constructor_template, v8::ObjectTemplate) \
427+
V(http2stream_constructor_template, v8::FunctionTemplate) \
428428
V(http2ping_constructor_template, v8::ObjectTemplate) \
429429
V(i18n_converter_template, v8::ObjectTemplate) \
430430
V(intervalhistogram_constructor_template, v8::FunctionTemplate) \
@@ -445,7 +445,7 @@
445445
V(pipe_constructor_template, v8::FunctionTemplate) \
446446
V(script_context_constructor_template, v8::FunctionTemplate) \
447447
V(secure_context_constructor_template, v8::FunctionTemplate) \
448-
V(shutdown_wrap_template, v8::ObjectTemplate) \
448+
V(shutdown_wrap_template, v8::FunctionTemplate) \
449449
V(soa_record_template, v8::DictionaryTemplate) \
450450
V(socketaddress_constructor_template, v8::FunctionTemplate) \
451451
V(space_stats_template, v8::DictionaryTemplate) \
@@ -464,7 +464,7 @@
464464
V(urlpatterncomponentresult_template, v8::DictionaryTemplate) \
465465
V(urlpatterninit_template, v8::DictionaryTemplate) \
466466
V(urlpatternresult_template, v8::DictionaryTemplate) \
467-
V(write_wrap_template, v8::ObjectTemplate) \
467+
V(write_wrap_template, v8::FunctionTemplate) \
468468
V(worker_cpu_profile_taker_template, v8::ObjectTemplate) \
469469
V(worker_cpu_usage_taker_template, v8::ObjectTemplate) \
470470
V(worker_heap_profile_taker_template, v8::ObjectTemplate) \

‎src/node_ffi.cc‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1276,6 +1276,7 @@ Local<FunctionTemplate> DynamicLibrary::GetConstructorTemplate(
12761276
static_cast<PropertyAttribute>(ReadOnly | DontDelete);
12771277

12781278
tmpl = NewFunctionTemplate(isolate, DynamicLibrary::New);
1279+
tmpl->SetClassName(FIXED_ONE_BYTE_STRING(isolate, "DynamicLibrary"));
12791280
tmpl->InstanceTemplate()->SetInternalFieldCount(
12801281
DynamicLibrary::kInternalFieldCount);
12811282
Local<Signature> signature = Signature::New(isolate, tmpl);
@@ -1347,7 +1348,11 @@ static void Initialize(Local<Object> target,
13471348

13481349
// Create the DynamicLibrary template
13491350
Local<FunctionTemplate> dl_tmpl = DynamicLibrary::GetConstructorTemplate(env);
1350-
SetConstructorFunction(context, target, "DynamicLibrary", dl_tmpl);
1351+
SetConstructorFunction(context,
1352+
target,
1353+
"DynamicLibrary",
1354+
dl_tmpl,
1355+
SetConstructorFunctionFlag::NONE);
13511356
SetMethod(context, target, "toString", ToString);
13521357
SetMethod(context, target, "toBuffer", ToBuffer);
13531358
SetMethod(context, target, "toArrayBuffer", ToArrayBuffer);

‎src/node_http2.cc‎

Lines changed: 39 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -2252,6 +2252,7 @@ Http2Stream* Http2Stream::New(Http2Session* session,
22522252
Local<Object> obj;
22532253
if (!session->env()
22542254
->http2stream_constructor_template()
2255+
->InstanceTemplate()
22552256
->NewInstance(session->env()->context())
22562257
.ToLocal(&obj)) {
22572258
return nullptr;
@@ -3568,35 +3569,44 @@ void Initialize(Local<Object> target,
35683569
SetMethod(context, target, "packSettings", PackSettings);
35693570
SetMethod(context, target, "setCallbackFunctions", SetCallbackFunctions);
35703571

3571-
Local<FunctionTemplate> ping = FunctionTemplate::New(env->isolate());
3572-
ping->SetClassName(FIXED_ONE_BYTE_STRING(env->isolate(), "Http2Ping"));
3573-
ping->Inherit(AsyncWrap::GetConstructorTemplate(env));
3574-
Local<ObjectTemplate> pingt = ping->InstanceTemplate();
3575-
pingt->SetInternalFieldCount(Http2Ping::kInternalFieldCount);
3576-
env->set_http2ping_constructor_template(pingt);
3577-
3578-
Local<FunctionTemplate> setting = FunctionTemplate::New(env->isolate());
3579-
setting->Inherit(AsyncWrap::GetConstructorTemplate(env));
3580-
Local<ObjectTemplate> settingt = setting->InstanceTemplate();
3581-
settingt->SetInternalFieldCount(Http2Settings::kInternalFieldCount);
3582-
env->set_http2settings_constructor_template(settingt);
3583-
3584-
Local<FunctionTemplate> stream = FunctionTemplate::New(env->isolate());
3585-
SetProtoMethod(isolate, stream, "id", Http2Stream::GetID);
3586-
SetProtoMethod(isolate, stream, "destroy", Http2Stream::Destroy);
3587-
SetProtoMethod(isolate, stream, "priority", Http2Stream::Priority);
3588-
SetProtoMethod(isolate, stream, "pushPromise", Http2Stream::PushPromise);
3589-
SetProtoMethod(isolate, stream, "info", Http2Stream::Info);
3590-
SetProtoMethod(isolate, stream, "trailers", Http2Stream::Trailers);
3591-
SetProtoMethod(isolate, stream, "respond", Http2Stream::Respond);
3592-
SetProtoMethod(isolate, stream, "rstStream", Http2Stream::RstStream);
3593-
SetProtoMethod(isolate, stream, "refreshState", Http2Stream::RefreshState);
3594-
stream->Inherit(AsyncWrap::GetConstructorTemplate(env));
3595-
StreamBase::AddMethods(env, stream);
3596-
Local<ObjectTemplate> streamt = stream->InstanceTemplate();
3597-
streamt->SetInternalFieldCount(Http2Stream::kInternalFieldCount);
3598-
env->set_http2stream_constructor_template(streamt);
3599-
SetConstructorFunction(context, target, "Http2Stream", stream);
3572+
if (env->http2ping_constructor_template().IsEmpty()) {
3573+
Local<FunctionTemplate> ping = FunctionTemplate::New(env->isolate());
3574+
ping->SetClassName(FIXED_ONE_BYTE_STRING(env->isolate(), "Http2Ping"));
3575+
ping->Inherit(AsyncWrap::GetConstructorTemplate(env));
3576+
Local<ObjectTemplate> pingt = ping->InstanceTemplate();
3577+
pingt->SetInternalFieldCount(Http2Ping::kInternalFieldCount);
3578+
env->set_http2ping_constructor_template(pingt);
3579+
}
3580+
3581+
if (env->http2settings_constructor_template().IsEmpty()) {
3582+
Local<FunctionTemplate> setting = FunctionTemplate::New(env->isolate());
3583+
setting->Inherit(AsyncWrap::GetConstructorTemplate(env));
3584+
Local<ObjectTemplate> settingt = setting->InstanceTemplate();
3585+
settingt->SetInternalFieldCount(Http2Settings::kInternalFieldCount);
3586+
env->set_http2settings_constructor_template(settingt);
3587+
}
3588+
3589+
Local<FunctionTemplate> stream = env->http2stream_constructor_template();
3590+
if (stream.IsEmpty()) {
3591+
stream = FunctionTemplate::New(env->isolate());
3592+
SetProtoMethod(isolate, stream, "id", Http2Stream::GetID);
3593+
SetProtoMethod(isolate, stream, "destroy", Http2Stream::Destroy);
3594+
SetProtoMethod(isolate, stream, "priority", Http2Stream::Priority);
3595+
SetProtoMethod(isolate, stream, "pushPromise", Http2Stream::PushPromise);
3596+
SetProtoMethod(isolate, stream, "info", Http2Stream::Info);
3597+
SetProtoMethod(isolate, stream, "trailers", Http2Stream::Trailers);
3598+
SetProtoMethod(isolate, stream, "respond", Http2Stream::Respond);
3599+
SetProtoMethod(isolate, stream, "rstStream", Http2Stream::RstStream);
3600+
SetProtoMethod(isolate, stream, "refreshState", Http2Stream::RefreshState);
3601+
stream->Inherit(AsyncWrap::GetConstructorTemplate(env));
3602+
StreamBase::AddMethods(env, stream);
3603+
stream->InstanceTemplate()->SetInternalFieldCount(
3604+
Http2Stream::kInternalFieldCount);
3605+
stream->SetClassName(FIXED_ONE_BYTE_STRING(isolate, "Http2Stream"));
3606+
env->set_http2stream_constructor_template(stream);
3607+
}
3608+
SetConstructorFunction(
3609+
context, target, "Http2Stream", stream, SetConstructorFunctionFlag::NONE);
36003610

36013611
Local<FunctionTemplate> session =
36023612
NewFunctionTemplate(isolate, Http2Session::New);

‎src/node_sqlite.cc‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5372,9 +5372,13 @@ static void Initialize(Local<Object> target,
53725372
SetConstructorFunction(context,
53735373
target,
53745374
"StatementSync",
5375-
StatementSync::GetConstructorTemplate(env));
5376-
SetConstructorFunction(
5377-
context, target, "Session", Session::GetConstructorTemplate(env));
5375+
StatementSync::GetConstructorTemplate(env),
5376+
SetConstructorFunctionFlag::NONE);
5377+
SetConstructorFunction(context,
5378+
target,
5379+
"Session",
5380+
Session::GetConstructorTemplate(env),
5381+
SetConstructorFunctionFlag::NONE);
53785382

53795383
target->Set(context, env->constants_string(), constants).Check();
53805384

‎src/pipe_wrap.cc‎

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -71,24 +71,28 @@ void PipeWrap::Initialize(Local<Object> target,
7171
Environment* env = Environment::GetCurrent(context);
7272
Isolate* isolate = env->isolate();
7373

74-
Local<FunctionTemplate> t = NewFunctionTemplate(isolate, New);
75-
t->InstanceTemplate()->SetInternalFieldCount(PipeWrap::kInternalFieldCount);
74+
Local<FunctionTemplate> t = env->pipe_constructor_template();
75+
if (t.IsEmpty()) {
76+
t = NewFunctionTemplate(isolate, New);
77+
t->InstanceTemplate()->SetInternalFieldCount(PipeWrap::kInternalFieldCount);
7678

77-
t->Inherit(LibuvStreamWrap::GetConstructorTemplate(env));
79+
t->Inherit(LibuvStreamWrap::GetConstructorTemplate(env));
7880

79-
SetProtoMethod(isolate, t, "bind", Bind);
80-
SetProtoMethod(isolate, t, "listen", Listen);
81-
SetProtoMethod(isolate, t, "connect", Connect);
82-
SetProtoMethod(isolate, t, "open", Open);
81+
SetProtoMethod(isolate, t, "bind", Bind);
82+
SetProtoMethod(isolate, t, "listen", Listen);
83+
SetProtoMethod(isolate, t, "connect", Connect);
84+
SetProtoMethod(isolate, t, "open", Open);
8385

8486
#ifdef _WIN32
85-
SetProtoMethod(isolate, t, "setPendingInstances", SetPendingInstances);
87+
SetProtoMethod(isolate, t, "setPendingInstances", SetPendingInstances);
8688
#endif
8789

88-
SetProtoMethod(isolate, t, "fchmod", Fchmod);
89-
90-
SetConstructorFunction(context, target, "Pipe", t);
91-
env->set_pipe_constructor_template(t);
90+
SetProtoMethod(isolate, t, "fchmod", Fchmod);
91+
t->SetClassName(FIXED_ONE_BYTE_STRING(isolate, "Pipe"));
92+
env->set_pipe_constructor_template(t);
93+
}
94+
SetConstructorFunction(
95+
context, target, "Pipe", t, SetConstructorFunctionFlag::NONE);
9296

9397
// Create FunctionTemplate for PipeConnectWrap.
9498
auto cwt = AsyncWrap::MakeLazilyInitializedJSTemplate(env);

‎src/stream_base.cc‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ int StreamBase::Shutdown(v8::Local<v8::Object> req_wrap_obj) {
4848

4949
if (req_wrap_obj.IsEmpty()) {
5050
if (!env->shutdown_wrap_template()
51+
->InstanceTemplate()
5152
->NewInstance(env->context())
5253
.ToLocal(&req_wrap_obj)) {
5354
return UV_EBUSY;
@@ -103,6 +104,7 @@ StreamWriteResult StreamBase::Write(uv_buf_t* bufs,
103104

104105
if (req_wrap_obj.IsEmpty()) {
105106
if (!env->write_wrap_template()
107+
->InstanceTemplate()
106108
->NewInstance(env->context())
107109
.ToLocal(&req_wrap_obj)) {
108110
return StreamWriteResult{false, UV_EBUSY, nullptr, 0, {}};
@@ -332,6 +334,7 @@ int StreamBase::WriteBuffer(const FunctionCallbackInfo<Value>& args) {
332334
if (lazy_req) {
333335
// Sending a handle requires a request object up front to reference it.
334336
if (!env->write_wrap_template()
337+
->InstanceTemplate()
335338
->NewInstance(env->context())
336339
.ToLocal(&req_wrap_obj)) {
337340
return UV_EBUSY;
@@ -447,6 +450,7 @@ int StreamBase::WriteString(const FunctionCallbackInfo<Value>& args) {
447450
if (lazy_req && req_wrap_obj.IsEmpty()) {
448451
// Sending a handle requires a request object up front to reference it.
449452
if (!env->write_wrap_template()
453+
->InstanceTemplate()
450454
->NewInstance(env->context())
451455
.ToLocal(&req_wrap_obj)) {
452456
return UV_EBUSY;

0 commit comments

Comments
 (0)