Skip to content

Commit ff05879

Browse files
codebyterecodebytere
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>
1 parent 7709fca commit ff05879

14 files changed

Lines changed: 231 additions & 154 deletions

‎src/crypto/crypto_keys.cc‎

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

1711-
Local<FunctionTemplate> t =
1712-
NewFunctionTemplate(isolate, NativeKeyObject::New);
1713-
t->InstanceTemplate()->SetInternalFieldCount(
1714-
NativeKeyObject::kInternalFieldCount);
1715-
CHECK(env->crypto_key_object_constructor_template().IsEmpty());
1716-
env->set_crypto_key_object_constructor_template(t);
1711+
Local<FunctionTemplate> t = env->crypto_key_object_constructor_template();
1712+
if (t.IsEmpty()) {
1713+
t = NewFunctionTemplate(isolate, NativeKeyObject::New);
1714+
t->InstanceTemplate()->SetInternalFieldCount(
1715+
NativeKeyObject::kInternalFieldCount);
1716+
env->set_crypto_key_object_constructor_template(t);
1717+
}
17171718

17181719
Local<Value> ctor;
17191720
if (!t->GetFunction(env->context()).ToLocal(&ctor))
@@ -1945,12 +1946,13 @@ void NativeCryptoKey::CreateCryptoKeyClass(
19451946
Local<Value> callback = args[0];
19461947
CHECK(callback->IsFunction());
19471948

1948-
Local<FunctionTemplate> t =
1949-
NewFunctionTemplate(isolate, NativeCryptoKey::New);
1950-
t->InstanceTemplate()->SetInternalFieldCount(
1951-
NativeCryptoKey::kInternalFieldCount);
1952-
CHECK(env->crypto_cryptokey_constructor_template().IsEmpty());
1953-
env->set_crypto_cryptokey_constructor_template(t);
1949+
Local<FunctionTemplate> t = env->crypto_cryptokey_constructor_template();
1950+
if (t.IsEmpty()) {
1951+
t = NewFunctionTemplate(isolate, NativeCryptoKey::New);
1952+
t->InstanceTemplate()->SetInternalFieldCount(
1953+
NativeCryptoKey::kInternalFieldCount);
1954+
env->set_crypto_cryptokey_constructor_template(t);
1955+
}
19541956

19551957
Local<Value> ctor;
19561958
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
@@ -106,8 +106,11 @@ Local<FunctionTemplate> DTLSContext::GetConstructorTemplate(Environment* env) {
106106
void DTLSContext::InitPerContext(Local<Object> target,
107107
Local<Context> context,
108108
Environment* env) {
109-
SetConstructorFunction(
110-
context, target, "DTLSContext", GetConstructorTemplate(env));
109+
SetConstructorFunction(context,
110+
target,
111+
"DTLSContext",
112+
GetConstructorTemplate(env),
113+
SetConstructorFunctionFlag::NONE);
111114
}
112115

113116
void DTLSContext::RegisterExternalReferences(

‎src/dtls/dtls_endpoint.cc‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -102,8 +102,11 @@ Local<FunctionTemplate> DTLSEndpoint::GetConstructorTemplate(Environment* env) {
102102
void DTLSEndpoint::InitPerContext(Local<Object> target,
103103
Local<Context> context,
104104
Environment* env) {
105-
SetConstructorFunction(
106-
context, target, "DTLSEndpoint", GetConstructorTemplate(env));
105+
SetConstructorFunction(context,
106+
target,
107+
"DTLSEndpoint",
108+
GetConstructorTemplate(env),
109+
SetConstructorFunctionFlag::NONE);
107110
}
108111

109112
void DTLSEndpoint::RegisterExternalReferences(

‎src/dtls/dtls_session.cc‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -131,8 +131,11 @@ Local<FunctionTemplate> DTLSSession::GetConstructorTemplate(Environment* env) {
131131
void DTLSSession::InitPerContext(Local<Object> target,
132132
Local<Context> context,
133133
Environment* env) {
134-
SetConstructorFunction(
135-
context, target, "DTLSSession", GetConstructorTemplate(env));
134+
SetConstructorFunction(context,
135+
target,
136+
"DTLSSession",
137+
GetConstructorTemplate(env),
138+
SetConstructorFunctionFlag::NONE);
136139
}
137140

138141
void DTLSSession::RegisterExternalReferences(

‎src/env_properties.h‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -453,7 +453,7 @@
453453
V(v8_heap_statistics_template, v8::DictionaryTemplate) \
454454
V(histogram_ctor_template, v8::FunctionTemplate) \
455455
V(http2settings_constructor_template, v8::ObjectTemplate) \
456-
V(http2stream_constructor_template, v8::ObjectTemplate) \
456+
V(http2stream_constructor_template, v8::FunctionTemplate) \
457457
V(http2ping_constructor_template, v8::ObjectTemplate) \
458458
V(i18n_converter_template, v8::ObjectTemplate) \
459459
V(intervalhistogram_constructor_template, v8::FunctionTemplate) \
@@ -474,7 +474,7 @@
474474
V(pipe_constructor_template, v8::FunctionTemplate) \
475475
V(script_context_constructor_template, v8::FunctionTemplate) \
476476
V(secure_context_constructor_template, v8::FunctionTemplate) \
477-
V(shutdown_wrap_template, v8::ObjectTemplate) \
477+
V(shutdown_wrap_template, v8::FunctionTemplate) \
478478
V(soa_record_template, v8::DictionaryTemplate) \
479479
V(socketaddress_constructor_template, v8::FunctionTemplate) \
480480
V(space_stats_template, v8::DictionaryTemplate) \
@@ -493,7 +493,7 @@
493493
V(urlpatterncomponentresult_template, v8::DictionaryTemplate) \
494494
V(urlpatterninit_template, v8::DictionaryTemplate) \
495495
V(urlpatternresult_template, v8::DictionaryTemplate) \
496-
V(write_wrap_template, v8::ObjectTemplate) \
496+
V(write_wrap_template, v8::FunctionTemplate) \
497497
V(worker_cpu_profile_taker_template, v8::ObjectTemplate) \
498498
V(worker_cpu_usage_taker_template, v8::ObjectTemplate) \
499499
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
@@ -1237,6 +1237,7 @@ Local<FunctionTemplate> DynamicLibrary::GetConstructorTemplate(
12371237
static_cast<PropertyAttribute>(ReadOnly | DontDelete);
12381238

12391239
tmpl = NewFunctionTemplate(isolate, DynamicLibrary::New);
1240+
tmpl->SetClassName(FIXED_ONE_BYTE_STRING(isolate, "DynamicLibrary"));
12401241
tmpl->InstanceTemplate()->SetInternalFieldCount(
12411242
DynamicLibrary::kInternalFieldCount);
12421243
Local<Signature> signature = Signature::New(isolate, tmpl);
@@ -1308,7 +1309,11 @@ static void Initialize(Local<Object> target,
13081309

13091310
// Create the DynamicLibrary template
13101311
Local<FunctionTemplate> dl_tmpl = DynamicLibrary::GetConstructorTemplate(env);
1311-
SetConstructorFunction(context, target, "DynamicLibrary", dl_tmpl);
1312+
SetConstructorFunction(context,
1313+
target,
1314+
"DynamicLibrary",
1315+
dl_tmpl,
1316+
SetConstructorFunctionFlag::NONE);
13121317
SetMethod(context, target, "toString", ToString);
13131318
SetMethod(context, target, "toBuffer", ToBuffer);
13141319
SetMethod(context, target, "toArrayBuffer", ToArrayBuffer);

‎src/node_http2.cc‎

Lines changed: 43 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -2264,6 +2264,7 @@ Http2Stream* Http2Stream::New(Http2Session* session,
22642264
Local<Object> obj;
22652265
if (!session->env()
22662266
->http2stream_constructor_template()
2267+
->InstanceTemplate()
22672268
->NewInstance(session->env()->context())
22682269
.ToLocal(&obj)) {
22692270
return nullptr;
@@ -3614,37 +3615,48 @@ void Initialize(Local<Object> target,
36143615
SetMethod(context, target, "packSettings", PackSettings);
36153616
SetMethod(context, target, "setCallbackFunctions", SetCallbackFunctions);
36163617

3617-
Local<FunctionTemplate> ping = FunctionTemplate::New(env->isolate());
3618-
ping->SetClassName(FIXED_ONE_BYTE_STRING(env->isolate(), "Http2Ping"));
3619-
ping->Inherit(AsyncWrap::GetConstructorTemplate(env));
3620-
Local<ObjectTemplate> pingt = ping->InstanceTemplate();
3621-
pingt->SetInternalFieldCount(Http2Ping::kInternalFieldCount);
3622-
env->set_http2ping_constructor_template(pingt);
3623-
3624-
Local<FunctionTemplate> setting = FunctionTemplate::New(env->isolate());
3625-
setting->Inherit(AsyncWrap::GetConstructorTemplate(env));
3626-
Local<ObjectTemplate> settingt = setting->InstanceTemplate();
3627-
settingt->SetInternalFieldCount(Http2Settings::kInternalFieldCount);
3628-
env->set_http2settings_constructor_template(settingt);
3629-
3630-
Local<FunctionTemplate> stream = FunctionTemplate::New(env->isolate());
3631-
SetProtoMethod(isolate, stream, "id", Http2Stream::GetID);
3632-
SetProtoMethod(isolate, stream, "destroy", Http2Stream::Destroy);
3633-
SetProtoMethod(isolate, stream, "priority", Http2Stream::Priority);
3634-
SetProtoMethod(isolate, stream, "pushPromise", Http2Stream::PushPromise);
3635-
SetProtoMethod(isolate, stream, "info", Http2Stream::Info);
3636-
SetProtoMethod(isolate, stream, "trailers", Http2Stream::Trailers);
3637-
SetProtoMethod(
3638-
isolate, stream, "disableAutoTrailers", Http2Stream::DisableAutoTrailers);
3639-
SetProtoMethod(isolate, stream, "respond", Http2Stream::Respond);
3640-
SetProtoMethod(isolate, stream, "rstStream", Http2Stream::RstStream);
3641-
SetProtoMethod(isolate, stream, "refreshState", Http2Stream::RefreshState);
3642-
stream->Inherit(AsyncWrap::GetConstructorTemplate(env));
3643-
StreamBase::AddMethods(env, stream);
3644-
Local<ObjectTemplate> streamt = stream->InstanceTemplate();
3645-
streamt->SetInternalFieldCount(Http2Stream::kInternalFieldCount);
3646-
env->set_http2stream_constructor_template(streamt);
3647-
SetConstructorFunction(context, target, "Http2Stream", stream);
3618+
if (env->http2ping_constructor_template().IsEmpty()) {
3619+
Local<FunctionTemplate> ping = FunctionTemplate::New(env->isolate());
3620+
ping->SetClassName(FIXED_ONE_BYTE_STRING(env->isolate(), "Http2Ping"));
3621+
ping->Inherit(AsyncWrap::GetConstructorTemplate(env));
3622+
Local<ObjectTemplate> pingt = ping->InstanceTemplate();
3623+
pingt->SetInternalFieldCount(Http2Ping::kInternalFieldCount);
3624+
env->set_http2ping_constructor_template(pingt);
3625+
}
3626+
3627+
if (env->http2settings_constructor_template().IsEmpty()) {
3628+
Local<FunctionTemplate> setting = FunctionTemplate::New(env->isolate());
3629+
setting->Inherit(AsyncWrap::GetConstructorTemplate(env));
3630+
Local<ObjectTemplate> settingt = setting->InstanceTemplate();
3631+
settingt->SetInternalFieldCount(Http2Settings::kInternalFieldCount);
3632+
env->set_http2settings_constructor_template(settingt);
3633+
}
3634+
3635+
Local<FunctionTemplate> stream = env->http2stream_constructor_template();
3636+
if (stream.IsEmpty()) {
3637+
stream = FunctionTemplate::New(env->isolate());
3638+
SetProtoMethod(isolate, stream, "id", Http2Stream::GetID);
3639+
SetProtoMethod(isolate, stream, "destroy", Http2Stream::Destroy);
3640+
SetProtoMethod(isolate, stream, "priority", Http2Stream::Priority);
3641+
SetProtoMethod(isolate, stream, "pushPromise", Http2Stream::PushPromise);
3642+
SetProtoMethod(isolate, stream, "info", Http2Stream::Info);
3643+
SetProtoMethod(isolate, stream, "trailers", Http2Stream::Trailers);
3644+
SetProtoMethod(isolate,
3645+
stream,
3646+
"disableAutoTrailers",
3647+
Http2Stream::DisableAutoTrailers);
3648+
SetProtoMethod(isolate, stream, "respond", Http2Stream::Respond);
3649+
SetProtoMethod(isolate, stream, "rstStream", Http2Stream::RstStream);
3650+
SetProtoMethod(isolate, stream, "refreshState", Http2Stream::RefreshState);
3651+
stream->Inherit(AsyncWrap::GetConstructorTemplate(env));
3652+
StreamBase::AddMethods(env, stream);
3653+
stream->InstanceTemplate()->SetInternalFieldCount(
3654+
Http2Stream::kInternalFieldCount);
3655+
stream->SetClassName(FIXED_ONE_BYTE_STRING(isolate, "Http2Stream"));
3656+
env->set_http2stream_constructor_template(stream);
3657+
}
3658+
SetConstructorFunction(
3659+
context, target, "Http2Stream", stream, SetConstructorFunctionFlag::NONE);
36483660

36493661
Local<FunctionTemplate> session =
36503662
NewFunctionTemplate(isolate, Http2Session::New);

‎src/node_sqlite.cc‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4632,9 +4632,13 @@ static void Initialize(Local<Object> target,
46324632
SetConstructorFunction(context,
46334633
target,
46344634
"StatementSync",
4635-
StatementSync::GetConstructorTemplate(env));
4636-
SetConstructorFunction(
4637-
context, target, "Session", Session::GetConstructorTemplate(env));
4635+
StatementSync::GetConstructorTemplate(env),
4636+
SetConstructorFunctionFlag::NONE);
4637+
SetConstructorFunction(context,
4638+
target,
4639+
"Session",
4640+
Session::GetConstructorTemplate(env),
4641+
SetConstructorFunctionFlag::NONE);
46384642

46394643
target->Set(context, env->constants_string(), constants).Check();
46404644

‎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)