From dd5dd67f1e50223d108af7831f57d6776c42a680 Mon Sep 17 00:00:00 2001 From: legendecas Date: Fri, 10 Nov 2023 00:20:59 +0800 Subject: [PATCH 1/6] src: make process binding data weak Avoid the realm being strongly referenced by the process binding data. PR-URL: https://github.com/nodejs/node/pull/48655 Reviewed-By: Matteo Collina Reviewed-By: Antoine du Hamel Reviewed-By: James M Snell Reviewed-By: Joyee Cheung --- lib/internal/process/per_thread.js | 5 +-- src/node_process.h | 18 +++++---- src/node_process_methods.cc | 62 +++++++++++++++++++----------- 3 files changed, 52 insertions(+), 33 deletions(-) diff --git a/lib/internal/process/per_thread.js b/lib/internal/process/per_thread.js index ce4822af019d5f..e88a18e75a667e 100644 --- a/lib/internal/process/per_thread.js +++ b/lib/internal/process/per_thread.js @@ -28,7 +28,6 @@ const { StringPrototypeStartsWith, Symbol, SymbolIterator, - Uint32Array, } = primordials; const { @@ -65,10 +64,10 @@ function refreshHrtimeBuffer() { // The 3 entries filled in by the original process.hrtime contains // the upper/lower 32 bits of the second part of the value, // and the remaining nanoseconds of the value. - hrValues = new Uint32Array(binding.hrtimeBuffer); + hrValues = binding.hrtimeBuffer; // Use a BigUint64Array in the closure because this is actually a bit // faster than simply returning a BigInt from C++ in V8 7.1. - hrBigintValues = new BigUint64Array(binding.hrtimeBuffer, 0, 1); + hrBigintValues = new BigUint64Array(binding.hrtimeBuffer.buffer, 0, 1); } // Create the buffers. diff --git a/src/node_process.h b/src/node_process.h index cb8c7962825f46..c911b3ddd7b78f 100644 --- a/src/node_process.h +++ b/src/node_process.h @@ -48,16 +48,20 @@ void PatchProcessObject(const v8::FunctionCallbackInfo& args); namespace process { class BindingData : public SnapshotableObject { public: + struct InternalFieldInfo : public node::InternalFieldInfoBase { + AliasedBufferIndex hrtime_buffer; + }; + static void AddMethods(v8::Isolate* isolate, v8::Local target); static void RegisterExternalReferences(ExternalReferenceRegistry* registry); - using InternalFieldInfo = InternalFieldInfoBase; - SERIALIZABLE_OBJECT_METHODS() SET_BINDING_ID(process_binding_data) - BindingData(Realm* realm, v8::Local object); + BindingData(Realm* realm, + v8::Local object, + InternalFieldInfo* info = nullptr); void MemoryInfo(MemoryTracker* tracker) const override; SET_MEMORY_INFO_NAME(BindingData) @@ -81,10 +85,10 @@ class BindingData : public SnapshotableObject { static void SlowBigInt(const v8::FunctionCallbackInfo& args); private: - static constexpr size_t kBufferSize = - std::max(sizeof(uint64_t), sizeof(uint32_t) * 3); - v8::Global array_buffer_; - std::shared_ptr backing_store_; + // Buffer length in uint32. + static constexpr size_t kHrTimeBufferLength = 3; + AliasedUint32Array hrtime_buffer_; + InternalFieldInfo* internal_field_info_ = nullptr; // These need to be static so that we have their addresses available to // register as external references in the snapshot at environment creation diff --git a/src/node_process_methods.cc b/src/node_process_methods.cc index 34d3c3af4c3e10..0342658c35ebdb 100644 --- a/src/node_process_methods.cc +++ b/src/node_process_methods.cc @@ -465,15 +465,29 @@ static void ReallyExit(const FunctionCallbackInfo& args) { namespace process { -BindingData::BindingData(Realm* realm, v8::Local object) - : SnapshotableObject(realm, object, type_int) { +BindingData::BindingData(Realm* realm, + v8::Local object, + InternalFieldInfo* info) + : SnapshotableObject(realm, object, type_int), + hrtime_buffer_(realm->isolate(), + kHrTimeBufferLength, + MAYBE_FIELD_PTR(info, hrtime_buffer)) { Isolate* isolate = realm->isolate(); Local context = realm->context(); - Local ab = ArrayBuffer::New(isolate, kBufferSize); - array_buffer_.Reset(isolate, ab); - object->Set(context, FIXED_ONE_BYTE_STRING(isolate, "hrtimeBuffer"), ab) - .ToChecked(); - backing_store_ = ab->GetBackingStore(); + + if (info == nullptr) { + object + ->Set(context, + FIXED_ONE_BYTE_STRING(isolate, "hrtimeBuffer"), + hrtime_buffer_.GetJSArray()) + .ToChecked(); + } else { + hrtime_buffer_.Deserialize(realm->context()); + } + + // The hrtime buffer is referenced from the binding data js object. + // Make the native handle weak to avoid keeping the realm alive. + hrtime_buffer_.MakeWeak(); } v8::CFunction BindingData::fast_number_(v8::CFunction::Make(FastNumber)); @@ -503,7 +517,7 @@ BindingData* BindingData::FromV8Value(Local value) { } void BindingData::MemoryInfo(MemoryTracker* tracker) const { - tracker->TrackField("array_buffer", array_buffer_); + tracker->TrackField("hrtime_buffer", hrtime_buffer_); } // This is the legacy version of hrtime before BigInt was introduced in @@ -516,20 +530,19 @@ void BindingData::MemoryInfo(MemoryTracker* tracker) const { // because there is no Uint64Array in JS. // The third entry contains the remaining nanosecond part of the value. void BindingData::NumberImpl(BindingData* receiver) { - // Make sure we don't accidentally access buffers wiped for snapshot. - CHECK(!receiver->array_buffer_.IsEmpty()); uint64_t t = uv_hrtime(); - uint32_t* fields = static_cast(receiver->backing_store_->Data()); - fields[0] = (t / NANOS_PER_SEC) >> 32; - fields[1] = (t / NANOS_PER_SEC) & 0xffffffff; - fields[2] = t % NANOS_PER_SEC; + receiver->hrtime_buffer_[0] = (t / NANOS_PER_SEC) >> 32; + receiver->hrtime_buffer_[1] = (t / NANOS_PER_SEC) & 0xffffffff; + receiver->hrtime_buffer_[2] = t % NANOS_PER_SEC; } void BindingData::BigIntImpl(BindingData* receiver) { - // Make sure we don't accidentally access buffers wiped for snapshot. - CHECK(!receiver->array_buffer_.IsEmpty()); uint64_t t = uv_hrtime(); - uint64_t* fields = static_cast(receiver->backing_store_->Data()); + // The buffer is a Uint32Array, so we need to reinterpret it as a + // Uint64Array to write the value. The buffer is valid at this scope so we + // can safely cast away the constness. + uint64_t* fields = reinterpret_cast( + const_cast(receiver->hrtime_buffer_.GetNativeBuffer())); fields[0] = t; } @@ -543,9 +556,10 @@ void BindingData::SlowNumber(const v8::FunctionCallbackInfo& args) { bool BindingData::PrepareForSerialization(Local context, v8::SnapshotCreator* creator) { - // It's not worth keeping. - // Release it, we will recreate it when the instance is dehydrated. - array_buffer_.Reset(); + DCHECK_NULL(internal_field_info_); + internal_field_info_ = InternalFieldInfoBase::New(type()); + internal_field_info_->hrtime_buffer = + hrtime_buffer_.Serialize(context, creator); // Return true because we need to maintain the reference to the binding from // JS land. return true; @@ -553,8 +567,8 @@ bool BindingData::PrepareForSerialization(Local context, InternalFieldInfoBase* BindingData::Serialize(int index) { DCHECK_IS_SNAPSHOT_SLOT(index); - InternalFieldInfo* info = - InternalFieldInfoBase::New(type()); + InternalFieldInfo* info = internal_field_info_; + internal_field_info_ = nullptr; return info; } @@ -566,7 +580,9 @@ void BindingData::Deserialize(Local context, v8::HandleScope scope(context->GetIsolate()); Realm* realm = Realm::GetCurrent(context); // Recreate the buffer in the constructor. - BindingData* binding = realm->AddBindingData(holder); + InternalFieldInfo* casted_info = static_cast(info); + BindingData* binding = + realm->AddBindingData(holder, casted_info); CHECK_NOT_NULL(binding); } From f09515b3aaac11b78ce8e9483fdd20922c7c4f03 Mon Sep 17 00:00:00 2001 From: legendecas Date: Fri, 10 Nov 2023 00:21:03 +0800 Subject: [PATCH 2/6] src: create worker per isolate properties PR-URL: https://github.com/nodejs/node/pull/48655 Reviewed-By: Matteo Collina Reviewed-By: Antoine du Hamel Reviewed-By: James M Snell Reviewed-By: Joyee Cheung --- src/node_binding.h | 5 ++-- src/node_messaging.cc | 70 ++++++++++++++++++++++++------------------- src/node_messaging.h | 2 +- 3 files changed, 44 insertions(+), 33 deletions(-) diff --git a/src/node_binding.h b/src/node_binding.h index 9f0692ca4e190b..b09b8a5e521b18 100644 --- a/src/node_binding.h +++ b/src/node_binding.h @@ -37,10 +37,11 @@ static_assert(static_cast(NM_F_LINKED) == V(contextify) \ V(encoding_binding) \ V(fs) \ + V(messaging) \ V(mksnapshot) \ - V(timers) \ - V(process_methods) \ V(performance) \ + V(process_methods) \ + V(timers) \ V(url) \ V(worker) \ NODE_BUILTIN_ICU_BINDINGS(V) diff --git a/src/node_messaging.cc b/src/node_messaging.cc index 04b998bb28ce4b..6c9a23e669b29d 100644 --- a/src/node_messaging.cc +++ b/src/node_messaging.cc @@ -30,6 +30,7 @@ using v8::Maybe; using v8::MaybeLocal; using v8::Nothing; using v8::Object; +using v8::ObjectTemplate; using v8::SharedArrayBuffer; using v8::SharedValueConveyor; using v8::String; @@ -707,7 +708,8 @@ MessagePort* MessagePort::New( std::unique_ptr data, std::shared_ptr sibling_group) { Context::Scope context_scope(context); - Local ctor_templ = GetMessagePortConstructorTemplate(env); + Local ctor_templ = + GetMessagePortConstructorTemplate(env->isolate_data()); // Construct a new instance, then assign the listener instance and possibly // the MessagePortData to it. @@ -1107,7 +1109,8 @@ void MessagePort::Stop(const FunctionCallbackInfo& args) { void MessagePort::CheckType(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); args.GetReturnValue().Set( - GetMessagePortConstructorTemplate(env)->HasInstance(args[0])); + GetMessagePortConstructorTemplate(env->isolate_data()) + ->HasInstance(args[0])); } void MessagePort::Drain(const FunctionCallbackInfo& args) { @@ -1184,28 +1187,30 @@ void MessagePort::MemoryInfo(MemoryTracker* tracker) const { tracker->TrackField("emit_message_fn", emit_message_fn_); } -Local GetMessagePortConstructorTemplate(Environment* env) { +Local GetMessagePortConstructorTemplate( + IsolateData* isolate_data) { // Factor generating the MessagePort JS constructor into its own piece // of code, because it is needed early on in the child environment setup. - Local templ = env->message_port_constructor_template(); + Local templ = + isolate_data->message_port_constructor_template(); if (!templ.IsEmpty()) return templ; { - Isolate* isolate = env->isolate(); + Isolate* isolate = isolate_data->isolate(); Local m = NewFunctionTemplate(isolate, MessagePort::New); - m->SetClassName(env->message_port_constructor_string()); + m->SetClassName(isolate_data->message_port_constructor_string()); m->InstanceTemplate()->SetInternalFieldCount( MessagePort::kInternalFieldCount); - m->Inherit(HandleWrap::GetConstructorTemplate(env)); + m->Inherit(HandleWrap::GetConstructorTemplate(isolate_data)); SetProtoMethod(isolate, m, "postMessage", MessagePort::PostMessage); SetProtoMethod(isolate, m, "start", MessagePort::Start); - env->set_message_port_constructor_template(m); + isolate_data->set_message_port_constructor_template(m); } - return GetMessagePortConstructorTemplate(env); + return GetMessagePortConstructorTemplate(isolate_data); } JSTransferable::JSTransferable(Environment* env, Local obj) @@ -1573,15 +1578,12 @@ static void BroadcastChannel(const FunctionCallbackInfo& args) { } } -static void InitMessaging(Local target, - Local unused, - Local context, - void* priv) { - Environment* env = Environment::GetCurrent(context); - Isolate* isolate = env->isolate(); +static void CreatePerIsolateProperties(IsolateData* isolate_data, + Local target) { + Isolate* isolate = isolate_data->isolate(); { - SetConstructorFunction(context, + SetConstructorFunction(isolate, target, "MessageChannel", NewFunctionTemplate(isolate, MessageChannel)); @@ -1594,31 +1596,36 @@ static void InitMessaging(Local target, JSTransferable::kInternalFieldCount); t->SetClassName(OneByteString(isolate, "JSTransferable")); SetConstructorFunction( - context, target, "JSTransferable", t, SetConstructorFunctionFlag::NONE); + isolate, target, "JSTransferable", t, SetConstructorFunctionFlag::NONE); } - SetConstructorFunction(context, + SetConstructorFunction(isolate, target, - env->message_port_constructor_string(), - GetMessagePortConstructorTemplate(env), - SetConstructorFunctionFlag::NONE); + isolate_data->message_port_constructor_string(), + GetMessagePortConstructorTemplate(isolate_data)); // These are not methods on the MessagePort prototype, because // the browser equivalents do not provide them. - SetMethod(context, target, "stopMessagePort", MessagePort::Stop); - SetMethod(context, target, "checkMessagePort", MessagePort::CheckType); - SetMethod(context, target, "drainMessagePort", MessagePort::Drain); + SetMethod(isolate, target, "stopMessagePort", MessagePort::Stop); + SetMethod(isolate, target, "checkMessagePort", MessagePort::CheckType); + SetMethod(isolate, target, "drainMessagePort", MessagePort::Drain); SetMethod( - context, target, "receiveMessageOnPort", MessagePort::ReceiveMessage); + isolate, target, "receiveMessageOnPort", MessagePort::ReceiveMessage); SetMethod( - context, target, "moveMessagePortToContext", MessagePort::MoveToContext); - SetMethod(context, + isolate, target, "moveMessagePortToContext", MessagePort::MoveToContext); + SetMethod(isolate, target, "setDeserializerCreateObjectFunction", SetDeserializerCreateObjectFunction); - SetMethod(context, target, "broadcastChannel", BroadcastChannel); - SetMethod(context, target, "structuredClone", StructuredClone); + SetMethod(isolate, target, "broadcastChannel", BroadcastChannel); + SetMethod(isolate, target, "structuredClone", StructuredClone); +} +static void CreatePerContextProperties(Local target, + Local unused, + Local context, + void* priv) { + Environment* env = Environment::GetCurrent(context); { Local domexception = GetDOMException(context).ToLocalChecked(); target @@ -1650,6 +1657,9 @@ static void RegisterExternalReferences(ExternalReferenceRegistry* registry) { } // namespace worker } // namespace node -NODE_BINDING_CONTEXT_AWARE_INTERNAL(messaging, node::worker::InitMessaging) +NODE_BINDING_CONTEXT_AWARE_INTERNAL(messaging, + node::worker::CreatePerContextProperties) +NODE_BINDING_PER_ISOLATE_INIT(messaging, + node::worker::CreatePerIsolateProperties) NODE_BINDING_EXTERNAL_REFERENCE(messaging, node::worker::RegisterExternalReferences) diff --git a/src/node_messaging.h b/src/node_messaging.h index 1c2a564d8d58e1..2207a7fd2a0ef9 100644 --- a/src/node_messaging.h +++ b/src/node_messaging.h @@ -366,7 +366,7 @@ class JSTransferable : public BaseObject { }; v8::Local GetMessagePortConstructorTemplate( - Environment* env); + IsolateData* isolate_data); } // namespace worker } // namespace node From 84af29b9996d0e9447096aa20dbc112b726b6bb4 Mon Sep 17 00:00:00 2001 From: legendecas Date: Fri, 10 Nov 2023 00:21:06 +0800 Subject: [PATCH 3/6] src: create fs_dir per isolate properties PR-URL: https://github.com/nodejs/node/pull/48655 Reviewed-By: Matteo Collina Reviewed-By: Antoine du Hamel Reviewed-By: James M Snell Reviewed-By: Joyee Cheung --- src/node_binding.h | 1 + src/node_dir.cc | 28 ++++++++++++++++------------ 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/src/node_binding.h b/src/node_binding.h index b09b8a5e521b18..2901231090cce3 100644 --- a/src/node_binding.h +++ b/src/node_binding.h @@ -37,6 +37,7 @@ static_assert(static_cast(NM_F_LINKED) == V(contextify) \ V(encoding_binding) \ V(fs) \ + V(fs_dir) \ V(messaging) \ V(mksnapshot) \ V(performance) \ diff --git a/src/node_dir.cc b/src/node_dir.cc index 10cde6067899c7..21d5b880ccee98 100644 --- a/src/node_dir.cc +++ b/src/node_dir.cc @@ -423,27 +423,29 @@ static void OpenDirSync(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(handle->object().As()); } -void Initialize(Local target, - Local unused, - Local context, - void* priv) { - Environment* env = Environment::GetCurrent(context); - Isolate* isolate = env->isolate(); +void CreatePerIsolateProperties(IsolateData* isolate_data, + Local target) { + Isolate* isolate = isolate_data->isolate(); - SetMethod(context, target, "opendir", OpenDir); - SetMethod(context, target, "opendirSync", OpenDirSync); + SetMethod(isolate, target, "opendir", OpenDir); + SetMethod(isolate, target, "opendirSync", OpenDirSync); // Create FunctionTemplate for DirHandle Local dir = NewFunctionTemplate(isolate, DirHandle::New); - dir->Inherit(AsyncWrap::GetConstructorTemplate(env)); + dir->Inherit(AsyncWrap::GetConstructorTemplate(isolate_data)); SetProtoMethod(isolate, dir, "read", DirHandle::Read); SetProtoMethod(isolate, dir, "close", DirHandle::Close); Local dirt = dir->InstanceTemplate(); dirt->SetInternalFieldCount(DirHandle::kInternalFieldCount); - SetConstructorFunction(context, target, "DirHandle", dir); - env->set_dir_instance_template(dirt); + SetConstructorFunction(isolate, target, "DirHandle", dir); + isolate_data->set_dir_instance_template(dirt); } +void CreatePerContextProperties(Local target, + Local unused, + Local context, + void* priv) {} + void RegisterExternalReferences(ExternalReferenceRegistry* registry) { registry->Register(OpenDir); registry->Register(OpenDirSync); @@ -456,6 +458,8 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) { } // end namespace node -NODE_BINDING_CONTEXT_AWARE_INTERNAL(fs_dir, node::fs_dir::Initialize) +NODE_BINDING_CONTEXT_AWARE_INTERNAL(fs_dir, + node::fs_dir::CreatePerContextProperties) +NODE_BINDING_PER_ISOLATE_INIT(fs_dir, node::fs_dir::CreatePerIsolateProperties) NODE_BINDING_EXTERNAL_REFERENCE(fs_dir, node::fs_dir::RegisterExternalReferences) From afa56f155639c92d3faaf21213063b7786699887 Mon Sep 17 00:00:00 2001 From: legendecas Date: Fri, 10 Nov 2023 00:21:08 +0800 Subject: [PATCH 4/6] src: create per isolate proxy env template PR-URL: https://github.com/nodejs/node/pull/48655 Reviewed-By: Matteo Collina Reviewed-By: Antoine du Hamel Reviewed-By: James M Snell Reviewed-By: Joyee Cheung --- src/env.cc | 1 + src/node_env_var.cc | 3 ++- src/node_process.h | 2 +- src/node_realm.cc | 5 +++-- 4 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/env.cc b/src/env.cc index 6349cd802a6210..d797fcd527f422 100644 --- a/src/env.cc +++ b/src/env.cc @@ -504,6 +504,7 @@ void IsolateData::CreateProperties() { binding::CreateInternalBindingTemplates(this); contextify::ContextifyContext::InitializeGlobalTemplates(this); + CreateEnvProxyTemplate(this); } constexpr uint16_t kDefaultCppGCEmebdderID = 0x90de; diff --git a/src/node_env_var.cc b/src/node_env_var.cc index 94936ea1c2bd22..bce7ae07214ddf 100644 --- a/src/node_env_var.cc +++ b/src/node_env_var.cc @@ -456,7 +456,8 @@ static void EnvDefiner(Local property, } } -void CreateEnvProxyTemplate(Isolate* isolate, IsolateData* isolate_data) { +void CreateEnvProxyTemplate(IsolateData* isolate_data) { + Isolate* isolate = isolate_data->isolate(); HandleScope scope(isolate); if (!isolate_data->env_proxy_template().IsEmpty()) return; Local env_proxy_ctor_template = diff --git a/src/node_process.h b/src/node_process.h index c911b3ddd7b78f..ee6e6a81676bd6 100644 --- a/src/node_process.h +++ b/src/node_process.h @@ -15,7 +15,7 @@ class MemoryTracker; class ExternalReferenceRegistry; class Realm; -void CreateEnvProxyTemplate(v8::Isolate* isolate, IsolateData* isolate_data); +void CreateEnvProxyTemplate(IsolateData* isolate_data); // Most of the time, it's best to use `console.error` to write // to the process.stderr stream. However, in some cases, such as diff --git a/src/node_realm.cc b/src/node_realm.cc index 7101b23e347c7c..23fb6bd55213ee 100644 --- a/src/node_realm.cc +++ b/src/node_realm.cc @@ -344,10 +344,11 @@ MaybeLocal PrincipalRealm::BootstrapRealm() { return MaybeLocal(); } + // Setup process.env proxy. Local env_string = FIXED_ONE_BYTE_STRING(isolate_, "env"); Local env_proxy; - CreateEnvProxyTemplate(isolate_, env_->isolate_data()); - if (!env_->env_proxy_template()->NewInstance(context()).ToLocal(&env_proxy) || + if (!isolate_data()->env_proxy_template()->NewInstance(context()).ToLocal( + &env_proxy) || process_object()->Set(context(), env_string, env_proxy).IsNothing()) { return MaybeLocal(); } From 25d434c9f317b1e38cfc389a9a23d683835747c7 Mon Sep 17 00:00:00 2001 From: legendecas Date: Fri, 10 Nov 2023 00:21:10 +0800 Subject: [PATCH 5/6] module: remove useCustomLoadersIfPresent flag The flag is always true and can be determined by isLoaderWorker solely. PR-URL: https://github.com/nodejs/node/pull/48655 Reviewed-By: Matteo Collina Reviewed-By: Antoine du Hamel Reviewed-By: James M Snell Reviewed-By: Joyee Cheung --- lib/internal/modules/esm/loader.js | 10 ++++------ lib/internal/process/esm_loader.js | 4 ++-- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js index 6044765c3709f5..d1ce6f1df1bceb 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -524,15 +524,13 @@ let emittedLoaderFlagWarning = false; * A loader instance is used as the main entry point for loading ES modules. Currently, this is a singleton; there is * only one used for loading the main module and everything in its dependency graph, though separate instances of this * class might be instantiated as part of bootstrap for other purposes. - * @param {boolean} useCustomLoadersIfPresent If the user has provided loaders via the --loader flag, use them. * @returns {ModuleLoader} */ -function createModuleLoader(useCustomLoadersIfPresent = true) { +function createModuleLoader() { let customizations = null; - if (useCustomLoadersIfPresent && - // Don't spawn a new worker if we're already in a worker thread created by instantiating CustomizedModuleLoader; - // doing so would cause an infinite loop. - !require('internal/modules/esm/utils').isLoaderWorker()) { + // Don't spawn a new worker if we're already in a worker thread created by instantiating CustomizedModuleLoader; + // doing so would cause an infinite loop. + if (!require('internal/modules/esm/utils').isLoaderWorker()) { const userLoaderPaths = getOptionValue('--experimental-loader'); if (userLoaderPaths.length > 0) { if (!emittedLoaderFlagWarning) { diff --git a/lib/internal/process/esm_loader.js b/lib/internal/process/esm_loader.js index 47c909c9e55435..0865d7ceef66b7 100644 --- a/lib/internal/process/esm_loader.js +++ b/lib/internal/process/esm_loader.js @@ -11,10 +11,10 @@ let esmLoader; module.exports = { get esmLoader() { - return esmLoader ??= createModuleLoader(true); + return esmLoader ??= createModuleLoader(); }, async loadESM(callback) { - esmLoader ??= createModuleLoader(true); + esmLoader ??= createModuleLoader(); try { const userImports = getOptionValue('--import'); if (userImports.length > 0) { From 401dfc27fbc3094a20288ce63a3aef311efd480b Mon Sep 17 00:00:00 2001 From: Chengzhong Wu Date: Fri, 10 Nov 2023 00:21:15 +0800 Subject: [PATCH 6/6] module: bootstrap module loaders in shadow realm This bootstraps ESM loaders in the ShadowRealm with `ShadowRealm.prototype.importValue` as its entry point and enables loading ESM and CJS modules in the ShadowRealm. The module is imported without a parent URL and resolved with the current process's working directory. PR-URL: https://github.com/nodejs/node/pull/48655 Reviewed-By: Matteo Collina Reviewed-By: Antoine du Hamel Reviewed-By: James M Snell Reviewed-By: Joyee Cheung --- lib/internal/bootstrap/realm.js | 13 +- lib/internal/bootstrap/shadow_realm.js | 21 ++ lib/internal/main/worker_thread.js | 1 + lib/internal/modules/esm/loader.js | 7 +- lib/internal/modules/esm/utils.js | 43 ++++- lib/internal/process/pre_execution.js | 38 +++- src/async_wrap.cc | 22 ++- src/env.cc | 11 +- src/module_wrap.cc | 182 ++++++++++-------- src/module_wrap.h | 13 +- src/node_binding.h | 1 + src/node_errors.h | 4 + src/node_shadow_realm.cc | 34 ++++ .../es-module-shadow-realm/custom-loaders.js | 15 ++ .../es-module-shadow-realm/preload-main.js | 9 + .../es-module-shadow-realm/preload.js | 1 + .../re-export-state-counter.mjs | 3 + .../es-module-shadow-realm/state-counter.mjs | 4 + ...st-shadow-realm-allowed-builtin-modules.js | 21 ++ .../test-shadow-realm-custom-loaders.js | 26 +++ test/parallel/test-shadow-realm-gc-module.js | 20 ++ .../test-shadow-realm-import-value-resolve.js | 28 +++ test/parallel/test-shadow-realm-module.js | 29 +++ .../test-shadow-realm-preload-module.js | 20 ++ 24 files changed, 444 insertions(+), 122 deletions(-) create mode 100644 lib/internal/bootstrap/shadow_realm.js create mode 100644 test/fixtures/es-module-shadow-realm/custom-loaders.js create mode 100644 test/fixtures/es-module-shadow-realm/preload-main.js create mode 100644 test/fixtures/es-module-shadow-realm/preload.js create mode 100644 test/fixtures/es-module-shadow-realm/re-export-state-counter.mjs create mode 100644 test/fixtures/es-module-shadow-realm/state-counter.mjs create mode 100644 test/parallel/test-shadow-realm-allowed-builtin-modules.js create mode 100644 test/parallel/test-shadow-realm-custom-loaders.js create mode 100644 test/parallel/test-shadow-realm-gc-module.js create mode 100644 test/parallel/test-shadow-realm-import-value-resolve.js create mode 100644 test/parallel/test-shadow-realm-module.js create mode 100644 test/parallel/test-shadow-realm-preload-module.js diff --git a/lib/internal/bootstrap/realm.js b/lib/internal/bootstrap/realm.js index c6935c6f7775fc..6034af9a36003c 100644 --- a/lib/internal/bootstrap/realm.js +++ b/lib/internal/bootstrap/realm.js @@ -50,6 +50,8 @@ const { ArrayFrom, + ArrayPrototypeFilter, + ArrayPrototypeIncludes, ArrayPrototypeMap, ArrayPrototypePush, ArrayPrototypeSlice, @@ -215,8 +217,8 @@ const internalBuiltinIds = builtinIds .filter((id) => StringPrototypeStartsWith(id, 'internal/') && id !== selfId); // When --expose-internals is on we'll add the internal builtin ids to these. -const canBeRequiredByUsersList = new SafeSet(publicBuiltinIds); -const canBeRequiredByUsersWithoutSchemeList = +let canBeRequiredByUsersList = new SafeSet(publicBuiltinIds); +let canBeRequiredByUsersWithoutSchemeList = new SafeSet(publicBuiltinIds.filter((id) => !schemelessBlockList.has(id))); /** @@ -269,6 +271,13 @@ class BuiltinModule { } } + static setRealmAllowRequireByUsers(ids) { + canBeRequiredByUsersList = + new SafeSet(ArrayPrototypeFilter(ids, (id) => ArrayPrototypeIncludes(publicBuiltinIds, id))); + canBeRequiredByUsersWithoutSchemeList = + new SafeSet(ArrayPrototypeFilter(ids, (id) => !schemelessBlockList.has(id))); + } + // To be called during pre-execution when --expose-internals is on. // Enables the user-land module loader to access internal modules. static exposeInternals() { diff --git a/lib/internal/bootstrap/shadow_realm.js b/lib/internal/bootstrap/shadow_realm.js new file mode 100644 index 00000000000000..b99502355d9818 --- /dev/null +++ b/lib/internal/bootstrap/shadow_realm.js @@ -0,0 +1,21 @@ +'use strict'; + +// This script sets up the context for shadow realms. + +const { + prepareShadowRealmExecution, +} = require('internal/process/pre_execution'); +const { + BuiltinModule, +} = require('internal/bootstrap/realm'); + +BuiltinModule.setRealmAllowRequireByUsers([ + /** + * The built-in modules exposed in the ShadowRealm must each be providing + * platform capabilities with no authority to cause side effects such as + * I/O or mutation of values that are shared across different realms within + * the same Node.js environment. + */ +]); + +prepareShadowRealmExecution(); diff --git a/lib/internal/main/worker_thread.js b/lib/internal/main/worker_thread.js index 12ae4a9b23212d..56697c3b2c2209 100644 --- a/lib/internal/main/worker_thread.js +++ b/lib/internal/main/worker_thread.js @@ -136,6 +136,7 @@ port.on('message', (message) => { const isLoaderWorker = doEval === 'internal' && filename === require('internal/modules/esm/utils').loaderWorkerId; + // Disable custom loaders in loader worker. setupUserModules(isLoaderWorker); if (!hasStdin) diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js index d1ce6f1df1bceb..0694c7ef2b902d 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -528,9 +528,10 @@ let emittedLoaderFlagWarning = false; */ function createModuleLoader() { let customizations = null; - // Don't spawn a new worker if we're already in a worker thread created by instantiating CustomizedModuleLoader; - // doing so would cause an infinite loop. - if (!require('internal/modules/esm/utils').isLoaderWorker()) { + // Don't spawn a new worker if custom loaders are disabled. For instance, if + // we're already in a worker thread created by instantiating + // CustomizedModuleLoader; doing so would cause an infinite loop. + if (!require('internal/modules/esm/utils').forceDefaultLoader()) { const userLoaderPaths = getOptionValue('--experimental-loader'); if (userLoaderPaths.length > 0) { if (!emittedLoaderFlagWarning) { diff --git a/lib/internal/modules/esm/utils.js b/lib/internal/modules/esm/utils.js index 8dee1a661b73ac..003ae7eb987f03 100644 --- a/lib/internal/modules/esm/utils.js +++ b/lib/internal/modules/esm/utils.js @@ -4,6 +4,7 @@ const { ArrayIsArray, SafeSet, SafeWeakMap, + Symbol, ObjectFreeze, } = primordials; @@ -157,6 +158,26 @@ function registerModule(referrer, registry) { moduleRegistries.set(idSymbol, registry); } +/** + * Registers the ModuleRegistry for dynamic import() calls with a realm + * as the referrer. Similar to {@link registerModule}, but this function + * generates a new id symbol instead of using the one from the referrer + * object. + * @param {globalThis} globalThis The globalThis object of the realm. + * @param {ModuleRegistry} registry + */ +function registerRealm(globalThis, registry) { + let idSymbol = globalThis[host_defined_option_symbol]; + // If the per-realm host-defined options is already registered, do nothing. + if (idSymbol) { + return; + } + // Otherwise, register the per-realm host-defined options. + idSymbol = Symbol('Realm globalThis'); + globalThis[host_defined_option_symbol] = idSymbol; + moduleRegistries.set(idSymbol, registry); +} + /** * Defines the `import.meta` object for a given module. * @param {symbol} symbol - Reference to the module. @@ -192,28 +213,29 @@ async function importModuleDynamicallyCallback(referrerSymbol, specifier, attrib throw new ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING(); } -let _isLoaderWorker = false; +let _forceDefaultLoader = false; /** * Initializes handling of ES modules. * This is configured during pre-execution. Specifically it's set to true for * the loader worker in internal/main/worker_thread.js. - * @param {boolean} [isLoaderWorker=false] - A boolean indicating whether the loader is a worker or not. + * @param {boolean} [forceDefaultLoader=false] - A boolean indicating disabling custom loaders. */ -function initializeESM(isLoaderWorker = false) { - _isLoaderWorker = isLoaderWorker; +function initializeESM(forceDefaultLoader = false) { + _forceDefaultLoader = forceDefaultLoader; initializeDefaultConditions(); - // Setup per-isolate callbacks that locate data or callbacks that we keep + // Setup per-realm callbacks that locate data or callbacks that we keep // track of for different ESM modules. setInitializeImportMetaObjectCallback(initializeImportMetaObject); setImportModuleDynamicallyCallback(importModuleDynamicallyCallback); } /** - * Determine whether the current process is a loader worker. - * @returns {boolean} Whether the current process is a loader worker. + * Determine whether custom loaders are disabled and it is forced to use the + * default loader. + * @returns {boolean} */ -function isLoaderWorker() { - return _isLoaderWorker; +function forceDefaultLoader() { + return _forceDefaultLoader; } /** @@ -253,10 +275,11 @@ async function initializeHooks() { module.exports = { registerModule, + registerRealm, initializeESM, initializeHooks, getDefaultConditions, getConditionsSet, loaderWorkerId: 'internal/modules/esm/worker', - isLoaderWorker, + forceDefaultLoader, }; diff --git a/lib/internal/process/pre_execution.js b/lib/internal/process/pre_execution.js index 917ba90a1c8bbb..9142fed75e9050 100644 --- a/lib/internal/process/pre_execution.js +++ b/lib/internal/process/pre_execution.js @@ -67,6 +67,26 @@ function prepareWorkerThreadExecution() { }); } +function prepareShadowRealmExecution() { + const { registerRealm } = require('internal/modules/esm/utils'); + // Patch the process object with legacy properties and normalizations. + // Do not expand argv1 as it is not available in ShadowRealm. + patchProcessObject(false); + setupDebugEnv(); + + // Disable custom loaders in ShadowRealm. + setupUserModules(true); + registerRealm(globalThis, { + __proto__: null, + importModuleDynamically: (specifier, _referrer, attributes) => { + // The handler for `ShadowRealm.prototype.importValue`. + const { esmLoader } = require('internal/process/esm_loader'); + // `parentURL` is not set in the case of a ShadowRealm top-level import. + return esmLoader.import(specifier, undefined, attributes); + }, + }); +} + function prepareExecution(options) { const { expandArgv1, initializeModules, isMainThread } = options; @@ -160,16 +180,17 @@ function setupSymbolDisposePolyfill() { } } -function setupUserModules(isLoaderWorker = false) { +function setupUserModules(forceDefaultLoader = false) { initializeCJSLoader(); - initializeESMLoader(isLoaderWorker); + initializeESMLoader(forceDefaultLoader); const CJSLoader = require('internal/modules/cjs/loader'); assert(!CJSLoader.hasLoadedAnyUserCJSModule); - // Loader workers are responsible for doing this themselves. - if (isLoaderWorker) { - return; + // Do not enable preload modules if custom loaders are disabled. + // For example, loader workers are responsible for doing this themselves. + // And preload modules are not supported in ShadowRealm as well. + if (!forceDefaultLoader) { + loadPreloadModules(); } - loadPreloadModules(); // Need to be done after --require setup. initializeFrozenIntrinsics(); } @@ -687,9 +708,9 @@ function initializeCJSLoader() { initializeCJS(); } -function initializeESMLoader(isLoaderWorker) { +function initializeESMLoader(forceDefaultLoader) { const { initializeESM } = require('internal/modules/esm/utils'); - initializeESM(isLoaderWorker); + initializeESM(forceDefaultLoader); // Patch the vm module when --experimental-vm-modules is on. // Please update the comments in vm.js when this block changes. @@ -765,6 +786,7 @@ module.exports = { setupUserModules, prepareMainThreadExecution, prepareWorkerThreadExecution, + prepareShadowRealmExecution, markBootstrapComplete, loadPreloadModules, initializeFrozenIntrinsics, diff --git a/src/async_wrap.cc b/src/async_wrap.cc index 42cddc52aed285..8b784cddf41603 100644 --- a/src/async_wrap.cc +++ b/src/async_wrap.cc @@ -372,8 +372,9 @@ void AsyncWrap::CreatePerContextProperties(Local target, Local unused, Local context, void* priv) { - Environment* env = Environment::GetCurrent(context); - Isolate* isolate = env->isolate(); + Realm* realm = Realm::GetCurrent(context); + Environment* env = realm->env(); + Isolate* isolate = realm->isolate(); HandleScope scope(isolate); PropertyAttribute ReadOnlyDontDelete = @@ -446,13 +447,16 @@ void AsyncWrap::CreatePerContextProperties(Local target, #undef FORCE_SET_TARGET_FIELD - env->set_async_hooks_init_function(Local()); - env->set_async_hooks_before_function(Local()); - env->set_async_hooks_after_function(Local()); - env->set_async_hooks_destroy_function(Local()); - env->set_async_hooks_promise_resolve_function(Local()); - env->set_async_hooks_callback_trampoline(Local()); - env->set_async_hooks_binding(target); + // TODO(legendecas): async hook functions are not realm-aware yet. + // This simply avoid overriding principal realm's functions when a + // ShadowRealm initializes the binding. + realm->set_async_hooks_init_function(Local()); + realm->set_async_hooks_before_function(Local()); + realm->set_async_hooks_after_function(Local()); + realm->set_async_hooks_destroy_function(Local()); + realm->set_async_hooks_promise_resolve_function(Local()); + realm->set_async_hooks_callback_trampoline(Local()); + realm->set_async_hooks_binding(target); } void AsyncWrap::RegisterExternalReferences( diff --git a/src/env.cc b/src/env.cc index d797fcd527f422..a429d5526d0af6 100644 --- a/src/env.cc +++ b/src/env.cc @@ -1654,10 +1654,13 @@ void AsyncHooks::MemoryInfo(MemoryTracker* tracker) const { void AsyncHooks::grow_async_ids_stack() { async_ids_stack_.reserve(async_ids_stack_.Length() * 3); - env()->async_hooks_binding()->Set( - env()->context(), - env()->async_ids_stack_string(), - async_ids_stack_.GetJSArray()).Check(); + env() + ->principal_realm() + ->async_hooks_binding() + ->Set(env()->context(), + env()->async_ids_stack_string(), + async_ids_stack_.GetJSArray()) + .Check(); } void AsyncHooks::FailWithCorruptedAsyncStack(double expected_async_id) { diff --git a/src/module_wrap.cc b/src/module_wrap.cc index 46241164bf8bd6..00ee4627f0f05a 100644 --- a/src/module_wrap.cc +++ b/src/module_wrap.cc @@ -39,6 +39,7 @@ using v8::MicrotaskQueue; using v8::Module; using v8::ModuleRequest; using v8::Object; +using v8::ObjectTemplate; using v8::PrimitiveArray; using v8::Promise; using v8::ScriptCompiler; @@ -49,15 +50,16 @@ using v8::UnboundModuleScript; using v8::Undefined; using v8::Value; -ModuleWrap::ModuleWrap(Environment* env, +ModuleWrap::ModuleWrap(Realm* realm, Local object, Local module, Local url, Local context_object, Local synthetic_evaluation_step) - : BaseObject(env, object), - module_(env->isolate(), module), + : BaseObject(realm, object), + module_(realm->isolate(), module), module_hash_(module->GetIdentityHash()) { + realm->env()->hash_to_module_map.emplace(module_hash_, this); object->SetInternalFieldForNodeCore(kModuleSlot, module); object->SetInternalField(kURLSlot, url); object->SetInternalField(kSyntheticEvaluationStepsSlot, @@ -72,7 +74,6 @@ ModuleWrap::ModuleWrap(Environment* env, } ModuleWrap::~ModuleWrap() { - HandleScope scope(env()->isolate()); auto range = env()->hash_to_module_map.equal_range(module_hash_); for (auto it = range.first; it != range.second; ++it) { if (it->second == this) { @@ -107,8 +108,8 @@ void ModuleWrap::New(const FunctionCallbackInfo& args) { CHECK(args.IsConstructCall()); CHECK_GE(args.Length(), 3); - Environment* env = Environment::GetCurrent(args); - Isolate* isolate = env->isolate(); + Realm* realm = Realm::GetCurrent(args); + Isolate* isolate = realm->isolate(); Local that = args.This(); @@ -122,7 +123,7 @@ void ModuleWrap::New(const FunctionCallbackInfo& args) { } else { CHECK(args[1]->IsObject()); contextify_context = ContextifyContext::ContextFromContextifiedSandbox( - env, args[1].As()); + realm->env(), args[1].As()); CHECK_NOT_NULL(contextify_context); context = contextify_context->context(); } @@ -148,8 +149,8 @@ void ModuleWrap::New(const FunctionCallbackInfo& args) { Local id_symbol = Symbol::New(isolate, url); host_defined_options->Set(isolate, HostDefinedOptions::kID, id_symbol); - ShouldNotAbortOnUncaughtScope no_abort_scope(env); - TryCatchScope try_catch(env); + ShouldNotAbortOnUncaughtScope no_abort_scope(realm->env()); + TryCatchScope try_catch(realm->env()); Local module; @@ -206,7 +207,9 @@ void ModuleWrap::New(const FunctionCallbackInfo& args) { if (try_catch.HasCaught() && !try_catch.HasTerminated()) { CHECK(!try_catch.Message().IsEmpty()); CHECK(!try_catch.Exception().IsEmpty()); - AppendExceptionLine(env, try_catch.Exception(), try_catch.Message(), + AppendExceptionLine(realm->env(), + try_catch.Exception(), + try_catch.Message(), ErrorHandlingMode::MODULE_ERROR); try_catch.ReThrow(); } @@ -215,18 +218,21 @@ void ModuleWrap::New(const FunctionCallbackInfo& args) { if (options == ScriptCompiler::kConsumeCodeCache && source.GetCachedData()->rejected) { THROW_ERR_VM_MODULE_CACHED_DATA_REJECTED( - env, "cachedData buffer was rejected"); + realm, "cachedData buffer was rejected"); try_catch.ReThrow(); return; } } } - if (!that->Set(context, env->url_string(), url).FromMaybe(false)) { + if (!that->Set(context, realm->isolate_data()->url_string(), url) + .FromMaybe(false)) { return; } - if (that->SetPrivate(context, env->host_defined_option_symbol(), id_symbol) + if (that->SetPrivate(context, + realm->isolate_data()->host_defined_option_symbol(), + id_symbol) .IsNothing()) { return; } @@ -236,28 +242,26 @@ void ModuleWrap::New(const FunctionCallbackInfo& args) { // be stored in an internal field. Local context_object = context->GetExtrasBindingObject(); Local synthetic_evaluation_step = - synthetic ? args[3] : Undefined(env->isolate()).As(); + synthetic ? args[3] : Undefined(realm->isolate()).As(); ModuleWrap* obj = new ModuleWrap( - env, that, module, url, context_object, synthetic_evaluation_step); + realm, that, module, url, context_object, synthetic_evaluation_step); obj->contextify_context_ = contextify_context; - env->hash_to_module_map.emplace(module->GetIdentityHash(), obj); - that->SetIntegrityLevel(context, IntegrityLevel::kFrozen); args.GetReturnValue().Set(that); } static Local createImportAttributesContainer( - Environment* env, Isolate* isolate, Local raw_attributes) { + Realm* realm, Isolate* isolate, Local raw_attributes) { Local attributes = - Object::New(isolate, v8::Null(env->isolate()), nullptr, nullptr, 0); + Object::New(isolate, v8::Null(isolate), nullptr, nullptr, 0); for (int i = 0; i < raw_attributes->Length(); i += 3) { attributes - ->Set(env->context(), - raw_attributes->Get(env->context(), i).As(), - raw_attributes->Get(env->context(), i + 1).As()) + ->Set(realm->context(), + raw_attributes->Get(realm->context(), i).As(), + raw_attributes->Get(realm->context(), i + 1).As()) .ToChecked(); } @@ -265,7 +269,7 @@ static Local createImportAttributesContainer( } void ModuleWrap::Link(const FunctionCallbackInfo& args) { - Environment* env = Environment::GetCurrent(args); + Realm* realm = Realm::GetCurrent(args); Isolate* isolate = args.GetIsolate(); CHECK_EQ(args.Length(), 1); @@ -292,14 +296,14 @@ void ModuleWrap::Link(const FunctionCallbackInfo& args) { // call the dependency resolve callbacks for (int i = 0; i < module_requests_length; i++) { Local module_request = - module_requests->Get(env->context(), i).As(); + module_requests->Get(realm->context(), i).As(); Local specifier = module_request->GetSpecifier(); - Utf8Value specifier_utf8(env->isolate(), specifier); + Utf8Value specifier_utf8(realm->isolate(), specifier); std::string specifier_std(*specifier_utf8, specifier_utf8.length()); Local raw_attributes = module_request->GetImportAssertions(); Local attributes = - createImportAttributesContainer(env, isolate, raw_attributes); + createImportAttributesContainer(realm, isolate, raw_attributes); Local argv[] = { specifier, @@ -315,11 +319,11 @@ void ModuleWrap::Link(const FunctionCallbackInfo& args) { maybe_resolve_return_value.ToLocalChecked(); if (!resolve_return_value->IsPromise()) { THROW_ERR_VM_MODULE_LINK_FAILURE( - env, "request for '%s' did not return promise", specifier_std); + realm, "request for '%s' did not return promise", specifier_std); return; } Local resolve_promise = resolve_return_value.As(); - obj->resolve_cache_[specifier_std].Reset(env->isolate(), resolve_promise); + obj->resolve_cache_[specifier_std].Reset(isolate, resolve_promise); promises[i] = resolve_promise; } @@ -329,13 +333,13 @@ void ModuleWrap::Link(const FunctionCallbackInfo& args) { } void ModuleWrap::Instantiate(const FunctionCallbackInfo& args) { - Environment* env = Environment::GetCurrent(args); + Realm* realm = Realm::GetCurrent(args); Isolate* isolate = args.GetIsolate(); ModuleWrap* obj; ASSIGN_OR_RETURN_UNWRAP(&obj, args.This()); Local context = obj->context(); Local module = obj->module_.Get(isolate); - TryCatchScope try_catch(env); + TryCatchScope try_catch(realm->env()); USE(module->InstantiateModule(context, ResolveModuleCallback)); // clear resolve cache on instantiate @@ -344,7 +348,9 @@ void ModuleWrap::Instantiate(const FunctionCallbackInfo& args) { if (try_catch.HasCaught() && !try_catch.HasTerminated()) { CHECK(!try_catch.Message().IsEmpty()); CHECK(!try_catch.Exception().IsEmpty()); - AppendExceptionLine(env, try_catch.Exception(), try_catch.Message(), + AppendExceptionLine(realm->env(), + try_catch.Exception(), + try_catch.Message(), ErrorHandlingMode::MODULE_ERROR); try_catch.ReThrow(); return; @@ -352,8 +358,8 @@ void ModuleWrap::Instantiate(const FunctionCallbackInfo& args) { } void ModuleWrap::Evaluate(const FunctionCallbackInfo& args) { - Environment* env = Environment::GetCurrent(args); - Isolate* isolate = env->isolate(); + Realm* realm = Realm::GetCurrent(args); + Isolate* isolate = realm->isolate(); ModuleWrap* obj; ASSIGN_OR_RETURN_UNWRAP(&obj, args.This()); Local context = obj->context(); @@ -368,14 +374,14 @@ void ModuleWrap::Evaluate(const FunctionCallbackInfo& args) { CHECK_EQ(args.Length(), 2); CHECK(args[0]->IsNumber()); - int64_t timeout = args[0]->IntegerValue(env->context()).FromJust(); + int64_t timeout = args[0]->IntegerValue(realm->context()).FromJust(); CHECK(args[1]->IsBoolean()); bool break_on_sigint = args[1]->IsTrue(); - ShouldNotAbortOnUncaughtScope no_abort_scope(env); - TryCatchScope try_catch(env); - Isolate::SafeForTerminationScope safe_for_termination(env->isolate()); + ShouldNotAbortOnUncaughtScope no_abort_scope(realm->env()); + TryCatchScope try_catch(realm->env()); + Isolate::SafeForTerminationScope safe_for_termination(isolate); bool timed_out = false; bool received_signal = false; @@ -406,16 +412,15 @@ void ModuleWrap::Evaluate(const FunctionCallbackInfo& args) { // Convert the termination exception into a regular exception. if (timed_out || received_signal) { - if (!env->is_main_thread() && env->is_stopping()) - return; - env->isolate()->CancelTerminateExecution(); + if (!realm->env()->is_main_thread() && realm->env()->is_stopping()) return; + isolate->CancelTerminateExecution(); // It is possible that execution was terminated by another timeout in // which this timeout is nested, so check whether one of the watchdogs // from this invocation is responsible for termination. if (timed_out) { - THROW_ERR_SCRIPT_EXECUTION_TIMEOUT(env, timeout); + THROW_ERR_SCRIPT_EXECUTION_TIMEOUT(realm->env(), timeout); } else if (received_signal) { - THROW_ERR_SCRIPT_EXECUTION_INTERRUPTED(env); + THROW_ERR_SCRIPT_EXECUTION_INTERRUPTED(realm->env()); } } @@ -429,7 +434,7 @@ void ModuleWrap::Evaluate(const FunctionCallbackInfo& args) { } void ModuleWrap::GetNamespace(const FunctionCallbackInfo& args) { - Environment* env = Environment::GetCurrent(args); + Realm* realm = Realm::GetCurrent(args); Isolate* isolate = args.GetIsolate(); ModuleWrap* obj; ASSIGN_OR_RETURN_UNWRAP(&obj, args.This()); @@ -439,7 +444,7 @@ void ModuleWrap::GetNamespace(const FunctionCallbackInfo& args) { switch (module->GetStatus()) { case v8::Module::Status::kUninstantiated: case v8::Module::Status::kInstantiating: - return env->ThrowError( + return realm->env()->ThrowError( "cannot get namespace, module has not been instantiated"); case v8::Module::Status::kInstantiated: case v8::Module::Status::kEvaluating: @@ -466,11 +471,11 @@ void ModuleWrap::GetStatus(const FunctionCallbackInfo& args) { void ModuleWrap::GetStaticDependencySpecifiers( const FunctionCallbackInfo& args) { - Environment* env = Environment::GetCurrent(args); + Realm* realm = Realm::GetCurrent(args); ModuleWrap* obj; ASSIGN_OR_RETURN_UNWRAP(&obj, args.This()); - Local module = obj->module_.Get(env->isolate()); + Local module = obj->module_.Get(realm->isolate()); Local module_requests = module->GetModuleRequests(); int count = module_requests->Length(); @@ -479,12 +484,12 @@ void ModuleWrap::GetStaticDependencySpecifiers( for (int i = 0; i < count; i++) { Local module_request = - module_requests->Get(env->context(), i).As(); + module_requests->Get(realm->context(), i).As(); specifiers[i] = module_request->GetSpecifier(); } args.GetReturnValue().Set( - Array::New(env->isolate(), specifiers.out(), count)); + Array::New(realm->isolate(), specifiers.out(), count)); } void ModuleWrap::GetError(const FunctionCallbackInfo& args) { @@ -501,15 +506,13 @@ MaybeLocal ModuleWrap::ResolveModuleCallback( Local specifier, Local import_attributes, Local referrer) { + Isolate* isolate = context->GetIsolate(); Environment* env = Environment::GetCurrent(context); if (env == nullptr) { - Isolate* isolate = context->GetIsolate(); THROW_ERR_EXECUTION_ENVIRONMENT_NOT_AVAILABLE(isolate); return MaybeLocal(); } - Isolate* isolate = env->isolate(); - Utf8Value specifier_utf8(isolate, specifier); std::string specifier_std(*specifier_utf8, specifier_utf8.length()); @@ -559,11 +562,16 @@ static MaybeLocal ImportModuleDynamically( THROW_ERR_EXECUTION_ENVIRONMENT_NOT_AVAILABLE(isolate); return MaybeLocal(); } + Realm* realm = Realm::GetCurrent(context); + if (realm == nullptr) { + // Fallback to the principal realm if it's in a vm context. + realm = env->principal_realm(); + } EscapableHandleScope handle_scope(isolate); Local import_callback = - env->host_import_module_dynamically_callback(); + realm->host_import_module_dynamically_callback(); Local id; Local options = host_defined_options.As(); @@ -579,7 +587,7 @@ static MaybeLocal ImportModuleDynamically( } Local attributes = - createImportAttributesContainer(env, isolate, import_attributes); + createImportAttributesContainer(realm, isolate, import_attributes); Local import_args[] = { id, @@ -603,13 +611,13 @@ static MaybeLocal ImportModuleDynamically( void ModuleWrap::SetImportModuleDynamicallyCallback( const FunctionCallbackInfo& args) { Isolate* isolate = args.GetIsolate(); - Environment* env = Environment::GetCurrent(args); + Realm* realm = Realm::GetCurrent(args); HandleScope handle_scope(isolate); CHECK_EQ(args.Length(), 1); CHECK(args[0]->IsFunction()); Local import_callback = args[0].As(); - env->set_host_import_module_dynamically_callback(import_callback); + realm->set_host_import_module_dynamically_callback(import_callback); isolate->SetHostImportModuleDynamicallyCallback(ImportModuleDynamically); } @@ -624,10 +632,15 @@ void ModuleWrap::HostInitializeImportMetaObjectCallback( if (module_wrap == nullptr) { return; } + Realm* realm = Realm::GetCurrent(context); + if (realm == nullptr) { + // Fallback to the principal realm if it's in a vm context. + realm = env->principal_realm(); + } Local wrap = module_wrap->object(); Local callback = - env->host_initialize_import_meta_object_callback(); + realm->host_initialize_import_meta_object_callback(); Local id; if (!wrap->GetPrivate(context, env->host_defined_option_symbol()) .ToLocal(&id)) { @@ -637,7 +650,7 @@ void ModuleWrap::HostInitializeImportMetaObjectCallback( Local args[] = {id, meta}; TryCatchScope try_catch(env); USE(callback->Call( - context, Undefined(env->isolate()), arraysize(args), args)); + context, Undefined(realm->isolate()), arraysize(args), args)); if (try_catch.HasCaught() && !try_catch.HasTerminated()) { try_catch.ReThrow(); } @@ -645,13 +658,13 @@ void ModuleWrap::HostInitializeImportMetaObjectCallback( void ModuleWrap::SetInitializeImportMetaObjectCallback( const FunctionCallbackInfo& args) { - Environment* env = Environment::GetCurrent(args); - Isolate* isolate = env->isolate(); + Realm* realm = Realm::GetCurrent(args); + Isolate* isolate = realm->isolate(); CHECK_EQ(args.Length(), 1); CHECK(args[0]->IsFunction()); Local import_meta_callback = args[0].As(); - env->set_host_initialize_import_meta_object_callback(import_meta_callback); + realm->set_host_initialize_import_meta_object_callback(import_meta_callback); isolate->SetHostInitializeImportMetaObjectCallback( HostInitializeImportMetaObjectCallback); @@ -742,12 +755,9 @@ void ModuleWrap::CreateCachedData(const FunctionCallbackInfo& args) { } } -void ModuleWrap::Initialize(Local target, - Local unused, - Local context, - void* priv) { - Environment* env = Environment::GetCurrent(context); - Isolate* isolate = env->isolate(); +void ModuleWrap::CreatePerIsolateProperties(IsolateData* isolate_data, + Local target) { + Isolate* isolate = isolate_data->isolate(); Local tpl = NewFunctionTemplate(isolate, New); tpl->InstanceTemplate()->SetInternalFieldCount( @@ -767,28 +777,36 @@ void ModuleWrap::Initialize(Local target, "getStaticDependencySpecifiers", GetStaticDependencySpecifiers); - SetConstructorFunction(context, target, "ModuleWrap", tpl); + SetConstructorFunction(isolate, target, "ModuleWrap", tpl); - SetMethod(context, + SetMethod(isolate, target, "setImportModuleDynamicallyCallback", SetImportModuleDynamicallyCallback); - SetMethod(context, + SetMethod(isolate, target, "setInitializeImportMetaObjectCallback", SetInitializeImportMetaObjectCallback); +} +void ModuleWrap::CreatePerContextProperties(Local target, + Local unused, + Local context, + void* priv) { + Realm* realm = Realm::GetCurrent(context); + Isolate* isolate = realm->isolate(); #define V(name) \ - target->Set(context, \ - FIXED_ONE_BYTE_STRING(env->isolate(), #name), \ - Integer::New(env->isolate(), Module::Status::name)) \ - .FromJust() - V(kUninstantiated); - V(kInstantiating); - V(kInstantiated); - V(kEvaluating); - V(kEvaluated); - V(kErrored); + target \ + ->Set(context, \ + FIXED_ONE_BYTE_STRING(isolate, #name), \ + Integer::New(isolate, Module::Status::name)) \ + .FromJust() + V(kUninstantiated); + V(kInstantiating); + V(kInstantiated); + V(kEvaluating); + V(kEvaluated); + V(kErrored); #undef V } @@ -812,7 +830,9 @@ void ModuleWrap::RegisterExternalReferences( } // namespace loader } // namespace node -NODE_BINDING_CONTEXT_AWARE_INTERNAL(module_wrap, - node::loader::ModuleWrap::Initialize) +NODE_BINDING_CONTEXT_AWARE_INTERNAL( + module_wrap, node::loader::ModuleWrap::CreatePerContextProperties) +NODE_BINDING_PER_ISOLATE_INIT( + module_wrap, node::loader::ModuleWrap::CreatePerIsolateProperties) NODE_BINDING_EXTERNAL_REFERENCE( module_wrap, node::loader::ModuleWrap::RegisterExternalReferences) diff --git a/src/module_wrap.h b/src/module_wrap.h index 1fc801edced9c5..e17048357feca2 100644 --- a/src/module_wrap.h +++ b/src/module_wrap.h @@ -10,6 +10,7 @@ namespace node { +class IsolateData; class Environment; class ExternalReferenceRegistry; @@ -40,10 +41,12 @@ class ModuleWrap : public BaseObject { kInternalFieldCount }; - static void Initialize(v8::Local target, - v8::Local unused, - v8::Local context, - void* priv); + static void CreatePerIsolateProperties(IsolateData* isolate_data, + v8::Local target); + static void CreatePerContextProperties(v8::Local target, + v8::Local unused, + v8::Local context, + void* priv); static void RegisterExternalReferences(ExternalReferenceRegistry* registry); static void HostInitializeImportMetaObjectCallback( v8::Local context, @@ -66,7 +69,7 @@ class ModuleWrap : public BaseObject { } private: - ModuleWrap(Environment* env, + ModuleWrap(Realm* realm, v8::Local object, v8::Local module, v8::Local url, diff --git a/src/node_binding.h b/src/node_binding.h index 2901231090cce3..7256bf2bbcf732 100644 --- a/src/node_binding.h +++ b/src/node_binding.h @@ -40,6 +40,7 @@ static_assert(static_cast(NM_F_LINKED) == V(fs_dir) \ V(messaging) \ V(mksnapshot) \ + V(module_wrap) \ V(performance) \ V(process_methods) \ V(timers) \ diff --git a/src/node_errors.h b/src/node_errors.h index 569dafe82df83d..8591faab8eaf6f 100644 --- a/src/node_errors.h +++ b/src/node_errors.h @@ -125,6 +125,10 @@ void AppendExceptionLine(Environment* env, inline void THROW_##code( \ Environment* env, const char* format, Args&&... args) { \ THROW_##code(env->isolate(), format, std::forward(args)...); \ + } \ + template \ + inline void THROW_##code(Realm* realm, const char* format, Args&&... args) { \ + THROW_##code(realm->isolate(), format, std::forward(args)...); \ } ERRORS_WITH_CODE(V) #undef V diff --git a/src/node_shadow_realm.cc b/src/node_shadow_realm.cc index 1cf0a57617b8b2..e7199e18756521 100644 --- a/src/node_shadow_realm.cc +++ b/src/node_shadow_realm.cc @@ -1,6 +1,7 @@ #include "node_shadow_realm.h" #include "env-inl.h" #include "node_errors.h" +#include "node_process.h" namespace node { namespace shadow_realm { @@ -9,6 +10,8 @@ using v8::EscapableHandleScope; using v8::HandleScope; using v8::Local; using v8::MaybeLocal; +using v8::Object; +using v8::String; using v8::Value; using TryCatchScope = node::errors::TryCatchScope; @@ -16,6 +19,11 @@ using TryCatchScope = node::errors::TryCatchScope; // static ShadowRealm* ShadowRealm::New(Environment* env) { ShadowRealm* realm = new ShadowRealm(env); + // TODO(legendecas): required by node::PromiseRejectCallback. + // Remove this once promise rejection doesn't need to be handled across + // realms. + realm->context()->SetSecurityToken( + env->principal_realm()->context()->GetSecurityToken()); // We do not expect the realm bootstrapping to throw any // exceptions. If it does, exit the current Node.js instance. @@ -32,6 +40,10 @@ MaybeLocal HostCreateShadowRealmContextCallback( Local initiator_context) { Environment* env = Environment::GetCurrent(initiator_context); EscapableHandleScope scope(env->isolate()); + + // We do not expect the realm bootstrapping to throw any + // exceptions. If it does, exit the current Node.js instance. + TryCatchScope try_catch(env, TryCatchScope::CatchMode::kFatal); ShadowRealm* realm = ShadowRealm::New(env); if (realm != nullptr) { return scope.Escape(realm->context()); @@ -137,6 +149,28 @@ v8::MaybeLocal ShadowRealm::BootstrapRealm() { } } + // The process object is not exposed globally in ShadowRealm yet. + // However, the process properties need to be setup for built-in modules. + // Specifically, process.cwd() is needed by the ESM loader. + if (ExecuteBootstrapper( + "internal/bootstrap/switches/does_not_own_process_state") + .IsEmpty()) { + return MaybeLocal(); + } + + // Setup process.env proxy. + Local env_string = FIXED_ONE_BYTE_STRING(isolate_, "env"); + Local env_proxy; + if (!isolate_data()->env_proxy_template()->NewInstance(context()).ToLocal( + &env_proxy) || + process_object()->Set(context(), env_string, env_proxy).IsNothing()) { + return MaybeLocal(); + } + + if (ExecuteBootstrapper("internal/bootstrap/shadow_realm").IsEmpty()) { + return MaybeLocal(); + } + return v8::True(isolate_); } diff --git a/test/fixtures/es-module-shadow-realm/custom-loaders.js b/test/fixtures/es-module-shadow-realm/custom-loaders.js new file mode 100644 index 00000000000000..bf4402250cc4f8 --- /dev/null +++ b/test/fixtures/es-module-shadow-realm/custom-loaders.js @@ -0,0 +1,15 @@ +// This fixture is used to test that custom loaders are not enabled in the ShadowRealm. + +'use strict'; +const assert = require('assert'); + +async function workInChildProcess() { + // Assert that the process is running with a custom loader. + const moduleNamespace = await import('file:///42.mjs'); + assert.strictEqual(moduleNamespace.default, 42); + + const realm = new ShadowRealm(); + await assert.rejects(realm.importValue('file:///42.mjs', 'default'), TypeError); +} + +workInChildProcess(); diff --git a/test/fixtures/es-module-shadow-realm/preload-main.js b/test/fixtures/es-module-shadow-realm/preload-main.js new file mode 100644 index 00000000000000..4258b012ad6139 --- /dev/null +++ b/test/fixtures/es-module-shadow-realm/preload-main.js @@ -0,0 +1,9 @@ +// This fixture is used to test that --require preload modules are not enabled in the ShadowRealm. + +'use strict'; +const assert = require('assert'); + +assert.strictEqual(globalThis.preload, 42); +const realm = new ShadowRealm(); +const value = realm.evaluate(`globalThis.preload`); +assert.strictEqual(value, undefined); diff --git a/test/fixtures/es-module-shadow-realm/preload.js b/test/fixtures/es-module-shadow-realm/preload.js new file mode 100644 index 00000000000000..dbbcb65e5a17e7 --- /dev/null +++ b/test/fixtures/es-module-shadow-realm/preload.js @@ -0,0 +1 @@ +globalThis.preload = 42; diff --git a/test/fixtures/es-module-shadow-realm/re-export-state-counter.mjs b/test/fixtures/es-module-shadow-realm/re-export-state-counter.mjs new file mode 100644 index 00000000000000..50a6aa3fe1e5b1 --- /dev/null +++ b/test/fixtures/es-module-shadow-realm/re-export-state-counter.mjs @@ -0,0 +1,3 @@ +// This module verifies that the module specifier is resolved relative to the +// current module and not the current working directory in the ShadowRealm. +export { getCounter } from "./state-counter.mjs"; diff --git a/test/fixtures/es-module-shadow-realm/state-counter.mjs b/test/fixtures/es-module-shadow-realm/state-counter.mjs new file mode 100644 index 00000000000000..c547658bf455ba --- /dev/null +++ b/test/fixtures/es-module-shadow-realm/state-counter.mjs @@ -0,0 +1,4 @@ +let counter = 0; +export const getCounter = () => { + return counter++; +}; diff --git a/test/parallel/test-shadow-realm-allowed-builtin-modules.js b/test/parallel/test-shadow-realm-allowed-builtin-modules.js new file mode 100644 index 00000000000000..2aa550ac7bb55b --- /dev/null +++ b/test/parallel/test-shadow-realm-allowed-builtin-modules.js @@ -0,0 +1,21 @@ +// Flags: --experimental-shadow-realm +'use strict'; +const common = require('../common'); +const assert = require('assert'); + +async function main() { + // Verifies that builtin modules can not be imported in the ShadowRealm. + const realm = new ShadowRealm(); + // The error object created inside the ShadowRealm with the error code + // property is not copied on the realm boundary. Only the error message + // is copied. Simply check the error message here. + await assert.rejects(realm.importValue('fs', 'readFileSync'), { + message: /Cannot find package 'fs'/, + }); + // As above, we can only validate the error message, not the error code. + await assert.rejects(realm.importValue('node:fs', 'readFileSync'), { + message: /No such built-in module: node:fs/, + }); +} + +main().then(common.mustCall()); diff --git a/test/parallel/test-shadow-realm-custom-loaders.js b/test/parallel/test-shadow-realm-custom-loaders.js new file mode 100644 index 00000000000000..80cda74bb88940 --- /dev/null +++ b/test/parallel/test-shadow-realm-custom-loaders.js @@ -0,0 +1,26 @@ +'use strict'; +const common = require('../common'); +const fixtures = require('../common/fixtures'); +const assert = require('assert'); + +const commonArgs = [ + '--experimental-shadow-realm', + '--no-warnings', +]; + +async function main() { + // Verifies that custom loaders are not enabled in the ShadowRealm. + const child = await common.spawnPromisified(process.execPath, [ + ...commonArgs, + '--experimental-loader', + fixtures.fileURL('es-module-loaders', 'loader-resolve-shortcircuit.mjs'), + '--experimental-loader', + fixtures.fileURL('es-module-loaders', 'loader-load-foo-or-42.mjs'), + fixtures.path('es-module-shadow-realm', 'custom-loaders.js'), + ]); + + assert.strictEqual(child.stderr, ''); + assert.strictEqual(child.code, 0); +} + +main().then(common.mustCall()); diff --git a/test/parallel/test-shadow-realm-gc-module.js b/test/parallel/test-shadow-realm-gc-module.js new file mode 100644 index 00000000000000..7f822bdd52fe1d --- /dev/null +++ b/test/parallel/test-shadow-realm-gc-module.js @@ -0,0 +1,20 @@ +// Flags: --experimental-shadow-realm --max-old-space-size=20 +'use strict'; + +/** + * Verifying modules imported by ShadowRealm instances can be correctly + * garbage collected. + */ + +const common = require('../common'); +const fixtures = require('../common/fixtures'); + +async function main() { + const mod = fixtures.fileURL('es-module-shadow-realm', 'state-counter.mjs'); + for (let i = 0; i < 100; i++) { + const realm = new ShadowRealm(); + await realm.importValue(mod, 'getCounter'); + } +} + +main().then(common.mustCall()); diff --git a/test/parallel/test-shadow-realm-import-value-resolve.js b/test/parallel/test-shadow-realm-import-value-resolve.js new file mode 100644 index 00000000000000..ee1c17d67c12f1 --- /dev/null +++ b/test/parallel/test-shadow-realm-import-value-resolve.js @@ -0,0 +1,28 @@ +// Flags: --experimental-shadow-realm +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const path = require('path'); + +common.skipIfWorker('process.chdir is not supported in workers.'); + +async function main() { + const realm = new ShadowRealm(); + + const dirname = __dirname; + // Set process cwd to the parent directory of __dirname. + const cwd = path.dirname(dirname); + process.chdir(cwd); + // Hardcode the relative path to ensure the string is still a valid relative + // URL string. + const relativePath = './fixtures/es-module-shadow-realm/re-export-state-counter.mjs'; + + // Make sure that the module can not be resolved relative to __filename. + assert.throws(() => require.resolve(relativePath), { code: 'MODULE_NOT_FOUND' }); + + // Resolve relative to the current working directory. + const getCounter = await realm.importValue(relativePath, 'getCounter'); + assert.strictEqual(typeof getCounter, 'function'); +} + +main().then(common.mustCall()); diff --git a/test/parallel/test-shadow-realm-module.js b/test/parallel/test-shadow-realm-module.js new file mode 100644 index 00000000000000..bc0c2c04f69f2b --- /dev/null +++ b/test/parallel/test-shadow-realm-module.js @@ -0,0 +1,29 @@ +// Flags: --experimental-shadow-realm +'use strict'; +const common = require('../common'); +const fixtures = require('../common/fixtures'); +const assert = require('assert'); + +async function main() { + const realm = new ShadowRealm(); + const mod = fixtures.fileURL('es-module-shadow-realm', 'state-counter.mjs'); + const getCounter = await realm.importValue(mod, 'getCounter'); + assert.strictEqual(getCounter(), 0); + const getCounter1 = await realm.importValue(mod, 'getCounter'); + // Returned value is a newly wrapped function. + assert.notStrictEqual(getCounter, getCounter1); + // Verify that the module state is shared between two `importValue` calls. + assert.strictEqual(getCounter1(), 1); + assert.strictEqual(getCounter(), 2); + + const { getCounter: getCounterThisRealm } = await import(mod); + assert.notStrictEqual(getCounterThisRealm, getCounter); + // Verify that the module state is not shared between two realms. + assert.strictEqual(getCounterThisRealm(), 0); + assert.strictEqual(getCounter(), 3); + + // Verify that shadow realm rejects to import a non-existing module. + await assert.rejects(realm.importValue('non-exists', 'exports'), TypeError); +} + +main().then(common.mustCall()); diff --git a/test/parallel/test-shadow-realm-preload-module.js b/test/parallel/test-shadow-realm-preload-module.js new file mode 100644 index 00000000000000..ebd29c1c4a8b80 --- /dev/null +++ b/test/parallel/test-shadow-realm-preload-module.js @@ -0,0 +1,20 @@ +'use strict'; +const common = require('../common'); +const fixtures = require('../common/fixtures'); +const { spawnSyncAndExitWithoutError } = require('../common/child_process'); + +const commonArgs = [ + '--experimental-shadow-realm', +]; + +async function main() { + // Verifies that --require preload modules are not enabled in the ShadowRealm. + spawnSyncAndExitWithoutError(process.execPath, [ + ...commonArgs, + '--require', + fixtures.path('es-module-shadow-realm', 'preload.js'), + fixtures.path('es-module-shadow-realm', 'preload-main.js'), + ]); +} + +main().then(common.mustCall());