diff --git a/doc/api/embedding.md b/doc/api/embedding.md index dfb84b49ef9b..7098973c6c2a 100644 --- a/doc/api/embedding.md +++ b/doc/api/embedding.md @@ -95,13 +95,10 @@ to as `node::Environment`. Each `node::Environment` is associated with: `node::Environment`s that share a `node::IsolateData` also share its `uv_loop_t`. `node::FreeEnvironment()` runs that loop until the handles of the -`node::Environment` being freed have closed, and JavaScript execution is -disallowed on the whole `v8::Isolate` while it does, so pending timers, I/O -callbacks and thread pool completions that belong to other `node::Environment`s -on the same loop can run inside that call without being able to call into -JavaScript. `node::Environment`s that are freed independently of one another -should each use their own `uv_loop_t` and `node::IsolateData`, or the embedder -should make sure the others have no pending work when one of them is freed. +`node::Environment` being freed have closed. Timers, I/O callbacks and thread +pool completions of the other `node::Environment`s that become due in those +loop iterations run normally, including their JavaScript; only the +`node::Environment` being freed can no longer call into JavaScript. In order to set up a `v8::Isolate`, an `v8::ArrayBuffer::Allocator` needs to be provided. One possible choice is the default Node.js allocator, which diff --git a/src/api/callback.cc b/src/api/callback.cc index c3850fa4afef..ef0c754ce562 100644 --- a/src/api/callback.cc +++ b/src/api/callback.cc @@ -96,8 +96,7 @@ InternalCallbackScope::InternalCallbackScope( } Isolate* isolate = env->isolate(); - // See IsolateData::handle_cleanup_depth. - if (env->isolate_data()->handle_cleanup_depth > 0) allow_js_.emplace(isolate); + if (handle_cleanup_depth > 0) allow_js_.emplace(isolate); HandleScope handle_scope(isolate); Local current_context = isolate->GetCurrentContext(); diff --git a/src/api/environment.cc b/src/api/environment.cc index ee9da42b606b..b8963b0fbcad 100644 --- a/src/api/environment.cc +++ b/src/api/environment.cc @@ -1048,6 +1048,7 @@ Maybe InitializePrimordials(Local context, // in the first place. However, creating BuiltinLoader instances is // relatively cheap and all the scripts that we may want to run at // startup are always present in it. + // NOLINTNEXTLINE(runtime/thread_local) thread_local builtins::BuiltinLoader builtin_loader; // Primordials can always be just eagerly compiled. builtin_loader.SetEagerCompile(); diff --git a/src/crypto/crypto_context.cc b/src/crypto/crypto_context.cc index 823d87e7f204..9ad4b5abdd85 100644 --- a/src/crypto/crypto_context.cc +++ b/src/crypto/crypto_context.cc @@ -95,37 +95,27 @@ struct X509Less { }; using X509Set = std::set; -// Per-thread root cert store. See NewRootCertStore() on what it contains. -static thread_local DeleteFnPtr root_cert_store; -// If the user calls tls.setDefaultCACertificates() this will be used -// to hold the user-provided certificates, the root_cert_store and any new -// copy generated by NewRootCertStore() will then contain the certificates -// from this set. -static thread_local std::unique_ptr root_certs_from_users; -static thread_local bool has_cleanup_hook = false; - -static void CleanupRootCertStore(void*) { - root_cert_store.reset(); - root_certs_from_users.reset(); - has_cleanup_hook = false; -} - -static void EnsureRootCertStoreCleanupHook(Environment* env) { - if (env == nullptr || has_cleanup_hook) { - return; - } +struct RootCertStore { + // See NewRootCertStore() on what it contains. + DeleteFnPtr store; + // Set by tls.setDefaultCACertificates(). Once set, NewRootCertStore() + // copies these certificates instead of loading the defaults. + std::unique_ptr certs_from_users; +}; - env->AddCleanupHook(CleanupRootCertStore, nullptr); - has_cleanup_hook = true; +void FreeRootCertStore(RootCertStore* root_certs) { + delete root_certs; +} + +static RootCertStore* GetRootCertStore(Environment* env) { + if (!env->root_cert_store) env->root_cert_store.reset(new RootCertStore()); + return env->root_cert_store.get(); } X509_STORE* GetOrCreateRootCertStore(Environment* env) { - EnsureRootCertStoreCleanupHook(env); - if (root_cert_store != nullptr) { - return root_cert_store.get(); - } - root_cert_store.reset(NewRootCertStore(env)); - return root_cert_store.get(); + RootCertStore* root_certs = GetRootCertStore(env); + if (!root_certs->store) root_certs->store.reset(NewRootCertStore(env)); + return root_certs->store.get(); } // Takes a string or buffer and loads it into a BIO. @@ -1062,8 +1052,9 @@ X509_STORE* NewRootCertStore(Environment* env) { // If the root cert store is already reset by users through // tls.setDefaultCACertificates(), just create a copy from the // user-provided certificates. - if (root_certs_from_users != nullptr) { - for (const auto& cert : *root_certs_from_users) { + const auto& certs_from_users = GetRootCertStore(env)->certs_from_users; + if (certs_from_users) { + for (const auto& cert : *certs_from_users) { CHECK_EQ(1, X509_STORE_add_cert(store, cert.get())); } return store; @@ -1230,12 +1221,13 @@ MaybeLocal X509sToArrayOfStrings(Environment* env, void GetUserRootCertificates(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); - CHECK_NOT_NULL(root_certs_from_users); + const auto& certs_from_users = GetRootCertStore(env)->certs_from_users; + CHECK(certs_from_users); Local results; if (X509sToArrayOfStrings(env, - root_certs_from_users->begin(), - root_certs_from_users->end(), - root_certs_from_users->size()) + certs_from_users->begin(), + certs_from_users->end(), + certs_from_users->size()) .ToLocal(&results)) { args.GetReturnValue().Set(results); } @@ -1246,12 +1238,12 @@ void ResetRootCertStore(const FunctionCallbackInfo& args) { CHECK(args[0]->IsArray()); Local cert_array = args[0].As(); Environment* env = Environment::GetCurrent(context); - EnsureRootCertStoreCleanupHook(env); + RootCertStore* root_certs = GetRootCertStore(env); if (cert_array->Length() == 0) { // If the array is empty, just clear the user certs and reset the store. - root_cert_store.reset(); - root_certs_from_users = std::make_unique(); + root_certs->store.reset(); + root_certs->certs_from_users = std::make_unique(); return; } @@ -1263,7 +1255,6 @@ void ResetRootCertStore(const FunctionCallbackInfo& args) { } if (certs->empty()) { - Environment* env = Environment::GetCurrent(context); return THROW_ERR_CRYPTO_OPERATION_FAILED( env, "No valid certificates found in the provided array"); } @@ -1275,11 +1266,11 @@ void ResetRootCertStore(const FunctionCallbackInfo& args) { // is not consumed by insert (element already exists). } - root_certs_from_users = std::move(new_set); + root_certs->certs_from_users = std::move(new_set); - // Reset the global root cert store so it will be recreated with the - // new certificates. - root_cert_store.reset(); + // Reset the root cert store so it will be recreated with the new + // certificates. + root_certs->store.reset(); } void GetSystemCACertificates(const FunctionCallbackInfo& args) { diff --git a/src/env.cc b/src/env.cc index 24cd7b62e4f0..3ba1579e0c66 100644 --- a/src/env.cc +++ b/src/env.cc @@ -649,7 +649,10 @@ IsolateData::IsolateData(Isolate* isolate, } } -IsolateData::~IsolateData() {} +IsolateData::~IsolateData() { + // FreeIsolateData() before FreeEnvironment() of an Environment using it. + CHECK_EQ(environment_count_, 0); +} // Deprecated API, embedders should use v8::Object::Wrap() directly instead. void SetCppgcReference(Isolate* isolate, @@ -981,6 +984,7 @@ Environment::Environment(IsolateData* isolate_data, ? AllocateEnvironmentThreadId().id : thread_id.id), thread_name_(thread_name) { + isolate_data->AddEnvironment(); #if HAVE_OPENSSL && NCRYPTO_USE_OPENSSL3_PROVIDER provider_digest_cache = std::make_unique(); provider_cipher_cache = std::make_unique(); @@ -1256,6 +1260,7 @@ Environment::~Environment() { // environment-owned methods before unloading any addon DSOs. provider_digest_cache.reset(); provider_cipher_cache.reset(); + root_cert_store.reset(); #if OPENSSL_WITH_EVP_MAC provider_mac_cache.reset(); #endif @@ -1273,6 +1278,7 @@ Environment::~Environment() { cpu_profiler_->Dispose(); cpu_profiler_ = nullptr; } + isolate_data_->RemoveEnvironment(); } void Environment::InitializeLibuv() { @@ -1436,6 +1442,9 @@ void Environment::ClosePerEnvHandles() { close_and_finish(reinterpret_cast(&task_queues_async_)); } +// NOLINTNEXTLINE(runtime/thread_local) +thread_local int handle_cleanup_depth = 0; + void Environment::CleanupHandles() { { Mutex::ScopedLock lock(native_immediates_threadsafe_mutex_); @@ -1453,8 +1462,8 @@ void Environment::CleanupHandles() { for (HandleWrap* handle : handle_wrap_queue_) handle->Close(); - isolate_data()->handle_cleanup_depth++; - auto done = OnScopeLeave([&]() { isolate_data()->handle_cleanup_depth--; }); + handle_cleanup_depth++; + auto done = OnScopeLeave([]() { handle_cleanup_depth--; }); while (handle_cleanup_waiting_ != 0 || request_waiting_ != 0 || !handle_wrap_queue_.IsEmpty()) { diff --git a/src/env.h b/src/env.h index 57f8369f5983..32acc81a00bb 100644 --- a/src/env.h +++ b/src/env.h @@ -76,6 +76,13 @@ class MacCache; namespace node { +#if HAVE_OPENSSL +namespace crypto { +struct RootCertStore; +void FreeRootCertStore(RootCertStore* root_certs); +} // namespace crypto +#endif // HAVE_OPENSSL + namespace shadow_realm { class ShadowRealm; } @@ -182,10 +189,8 @@ class NODE_EXTERN_PRIVATE IsolateData : public MemoryRetainer { inline worker::Worker* worker_context() const; inline void set_worker_context(worker::Worker* context); - // Non-zero while an Environment on this isolate is closing its handles with - // JS disallowed isolate-wide; InternalCallbackScope re-allows it for the - // other Environments whose callbacks run in those loop turns. - int handle_cleanup_depth = 0; + void AddEnvironment() { environment_count_++; } + void RemoveEnvironment() { environment_count_--; } #define VP(PropertyName, StringValue) V(v8::Private, PropertyName) #define VY(PropertyName, StringValue) V(v8::Symbol, PropertyName) @@ -288,6 +293,7 @@ class NODE_EXTERN_PRIVATE IsolateData : public MemoryRetainer { std::shared_ptr options_; worker::Worker* worker_context_ = nullptr; + size_t environment_count_ = 0; PerIsolateWrapperData* wrapper_data_; static Mutex isolate_data_mutex_; @@ -1212,6 +1218,7 @@ class Environment final : public MemoryRetainer { std::unique_ptr provider_mac_cache; std::vector supported_mac_algorithms; bool supported_mac_algorithms_initialized = false; + DeleteFnPtr root_cert_store; #endif // HAVE_OPENSSL v8::Global temporary_required_module_facade_original; diff --git a/src/inspector_agent.cc b/src/inspector_agent.cc index 9cdca9e1de21..a424e9e1a658 100644 --- a/src/inspector_agent.cc +++ b/src/inspector_agent.cc @@ -511,15 +511,8 @@ bool IsFilePath(const std::string& path) { #endif // __POSIX__ void ThrowUninitializedInspectorError(Environment* env) { - HandleScope scope(env->isolate()); - - std::string_view msg = - "This Environment was initialized without a V8::Inspector"; - Local exception; - if (ToV8Value(env->context(), msg, env->isolate()).ToLocal(&exception)) { - env->isolate()->ThrowException(exception); - } - // V8 will have scheduled a superseding error here. + THROW_ERR_INSPECTOR_NOT_AVAILABLE( + env, "This Environment was initialized without a V8::Inspector"); } } // namespace diff --git a/src/node.h b/src/node.h index 9018a4b39b02..6f39ed6f7f40 100644 --- a/src/node.h +++ b/src/node.h @@ -889,8 +889,8 @@ NODE_EXTERN v8::MaybeLocal LoadEnvironment( EmbedderPreloadCallback preload = nullptr); // Runs `env`'s event loop until its handles have closed, with JavaScript -// execution disallowed on the isolate; see doc/api/embedding.md if that loop -// is shared with other Environments. +// execution disallowed for `env`; see doc/api/embedding.md if that loop is +// shared with other Environments. NODE_EXTERN void FreeEnvironment(Environment* env); // Set a callback that is called when process.exit() is called from JS, diff --git a/src/node_binding.cc b/src/node_binding.cc index 568325e8496a..8cb5578eb048 100644 --- a/src/node_binding.cc +++ b/src/node_binding.cc @@ -180,6 +180,7 @@ struct dl_wrap { static Mutex dlhandles_mutex; static std::unordered_set dlhandles; +// NOLINTNEXTLINE(runtime/thread_local) static thread_local std::string dlerror_storage; char* wrapped_dlerror() { @@ -286,6 +287,7 @@ using v8::Value; // Globals per process static node_module* modlist_internal; static node_module* modlist_linked; +// NOLINTNEXTLINE(runtime/thread_local) static thread_local node_module* thread_local_modpending; // This is set by node::Init() which is used by embedders diff --git a/src/node_debug.cc b/src/node_debug.cc index c995e791f252..8efb049a76fc 100644 --- a/src/node_debug.cc +++ b/src/node_debug.cc @@ -24,8 +24,10 @@ using v8::Number; using v8::Object; using v8::Value; +// NOLINTNEXTLINE(runtime/thread_local) thread_local std::unordered_map generic_usage_counters; +// NOLINTNEXTLINE(runtime/thread_local) thread_local std::unordered_map v8_fast_api_call_counts; diff --git a/src/node_errors.cc b/src/node_errors.cc index cf000047d38d..cf71884c88db 100644 --- a/src/node_errors.cc +++ b/src/node_errors.cc @@ -190,9 +190,11 @@ static std::string GetErrorSource(Isolate* isolate, } static std::atomic is_in_oom{false}; +// NOLINTNEXTLINE(runtime/thread_local) static thread_local std::atomic is_retrieving_js_stacktrace{false}; // This is thread-local because it only guards re-entrancy within the current // thread's uncaught-exception path; no cross-thread synchronization is needed. +// NOLINTNEXTLINE(runtime/thread_local) static thread_local bool is_in_uncaught_exception = false; MaybeLocal GetCurrentStackTrace(Isolate* isolate, int frame_count) { if (isolate == nullptr) { diff --git a/src/node_internals.h b/src/node_internals.h index 1eacd95e3a42..bb45d73e9090 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -283,6 +283,12 @@ class InternalCallbackScope { std::optional allow_js_; }; +// Non-zero while an Environment on this thread is closing its handles with JS +// disallowed isolate-wide; InternalCallbackScope re-allows it for the other +// Environments whose callbacks run in those loop turns. +// NOLINTNEXTLINE(runtime/thread_local) +extern thread_local int handle_cleanup_depth; + class DebugSealHandleScope { public: explicit inline DebugSealHandleScope(v8::Isolate* isolate = nullptr) diff --git a/src/quic/README.md b/src/quic/README.md index 8c23ed2f4af9..acea22840a7e 100644 --- a/src/quic/README.md +++ b/src/quic/README.md @@ -150,28 +150,30 @@ The Application is selected as soon as the ALPN protocol is known: immediately for clients, and for servers from the `OnClientHello` TLS callback (see [Server handshake ordering](#server-handshake-ordering)). -### Thread-Local Allocator +### Allocator Both ngtcp2 and nghttp3 require custom allocators (`ngtcp2_mem`, `nghttp3_mem`). These allocator structs must outlive every object they create. Some nghttp3 objects (notably `rcbuf`s backing V8 external strings) can survive past `BindingData` destruction during isolate teardown. -The solution uses `thread_local` storage: +Each `BindingData` owns a heap-allocated `QuicAllocState` that holds both +allocator structs and counts live allocations: ```cpp struct QuicAllocState { - BindingData* binding = nullptr; // Nulled in ~BindingData + BindingData* binding; // Nulled in ~BindingData + size_t live_allocations = 0; ngtcp2_mem ngtcp2; nghttp3_mem nghttp3; }; -thread_local QuicAllocState quic_alloc_state; ``` Each allocation prepends its size before the returned pointer. This allows `free` and `realloc` to report correct sizes for memory tracking. When `binding` is null (after `BindingData` destruction), allocations still -succeed but memory tracking is silently skipped. +succeed but memory tracking is silently skipped. The state is deleted once +`binding` is null and the last allocation has been freed. ## Session Lifecycle diff --git a/src/quic/bindingdata.cc b/src/quic/bindingdata.cc index e73ae82e980d..49f6984d574b 100644 --- a/src/quic/bindingdata.cc +++ b/src/quic/bindingdata.cc @@ -37,33 +37,30 @@ using v8::Value; namespace quic { // ============================================================================ -// Thread-local QUIC allocator. +// QUIC allocator. // -// Both ngtcp2 and nghttp3 take an allocator struct (ngtcp2_mem / -// nghttp3_mem) whose pointer is stored inside every object they -// allocate. Some of those objects — notably nghttp3 rcbufs backing -// V8 external strings — can outlive the BindingData that created them -// (freed during V8 isolate teardown, after Environment cleanup). -// -// To handle this safely, both allocators live in a thread-local static -// struct that is never destroyed. Memory tracking goes through the -// BindingData pointer when it is alive and is silently skipped during -// teardown (after ~BindingData nulls the pointer). +// ngtcp2 and nghttp3 keep a pointer to their allocator struct in every object +// they allocate, and nghttp3 rcbufs backing V8 external strings can be freed +// after the BindingData is gone. A QuicAllocState is therefore deleted only +// once its BindingData has been destroyed and its last allocation freed. // // The allocation functions use the same prepended-size-header scheme as // NgLibMemoryManager (node_mem-inl.h) so that frees always know the // allocation size regardless of whether BindingData is still around. -namespace { struct QuicAllocState { - BindingData* binding = nullptr; + BindingData* binding; + size_t live_allocations = 0; ngtcp2_mem ngtcp2 = {}; nghttp3_mem nghttp3 = {}; + + void OnFreed() { + CHECK_GT(live_allocations, 0); + if (--live_allocations == 0 && binding == nullptr) delete this; + } }; -thread_local QuicAllocState quic_alloc_state; -// Core allocation functions shared by both ngtcp2 and nghttp3. -// user_data always points to the thread-local QuicAllocState. +namespace { void* QuicRealloc(void* ptr, size_t size, void* user_data) { auto* state = static_cast(user_data); @@ -78,6 +75,10 @@ void* QuicRealloc(void* ptr, size_t size, void* user_data) { previous_size = *reinterpret_cast(original_ptr); if (previous_size == 0) { char* ret = UncheckedRealloc(original_ptr, size); + if (size == 0) { + state->OnFreed(); + return nullptr; + } if (ret != nullptr) ret += kReserveSizeAndAlign; return ret; } @@ -96,6 +97,7 @@ void* QuicRealloc(void* ptr, size_t size, void* user_data) { state->binding->env()->external_memory_accounter()->Update( state->binding->env()->isolate(), new_size); } + if (ptr == nullptr) state->live_allocations++; *reinterpret_cast(mem) = size; mem += kReserveSizeAndAlign; } else if (size == 0) { @@ -104,6 +106,7 @@ void* QuicRealloc(void* ptr, size_t size, void* user_data) { state->binding->env()->external_memory_accounter()->Decrease( state->binding->env()->isolate(), previous_size); } + if (ptr != nullptr) state->OnFreed(); } return mem; } @@ -232,7 +235,8 @@ BindingData& BindingData::Get(Environment* env) { } BindingData::~BindingData() { - quic_alloc_state.binding = nullptr; + alloc_state_->binding = nullptr; + if (alloc_state_->live_allocations == 0) delete alloc_state_; // flush_check_ is cleaned up by ~CheckWrapHandle() after the destructor // body completes. The inner CheckWrap (and its uv_check_t) will be freed // later by the uv_close callback, after CleanupHandles() runs uv_run(). @@ -240,27 +244,11 @@ BindingData::~BindingData() { } ngtcp2_mem* BindingData::ngtcp2_allocator() { - quic_alloc_state.binding = this; - quic_alloc_state.ngtcp2 = { - &quic_alloc_state, - Ngtcp2Malloc, - Ngtcp2Free, - Ngtcp2Calloc, - Ngtcp2Realloc, - }; - return &quic_alloc_state.ngtcp2; + return &alloc_state_->ngtcp2; } nghttp3_mem* BindingData::nghttp3_allocator() { - quic_alloc_state.binding = this; - quic_alloc_state.nghttp3 = { - &quic_alloc_state, - Nghttp3Malloc, - Nghttp3Free, - Nghttp3Calloc, - Nghttp3Realloc, - }; - return &quic_alloc_state.nghttp3; + return &alloc_state_->nghttp3; } void BindingData::CheckAllocatedSize(size_t previous_size) const { @@ -349,7 +337,12 @@ JS_METHOD_IMPL(BindingData::SetHeadersInterest) { BindingData::BindingData(Realm* realm, Local object) : BaseObject(realm, object), + alloc_state_(new QuicAllocState{this}), flush_check_(env(), [this]() { OnFlushCheck(); }) { + alloc_state_->ngtcp2 = { + alloc_state_, Ngtcp2Malloc, Ngtcp2Free, Ngtcp2Calloc, Ngtcp2Realloc}; + alloc_state_->nghttp3 = { + alloc_state_, Nghttp3Malloc, Nghttp3Free, Nghttp3Calloc, Nghttp3Realloc}; MakeWeak(); // Unref so the check handle doesn't keep the event loop alive on its own. flush_check_.Unref(); diff --git a/src/quic/bindingdata.h b/src/quic/bindingdata.h index 2ef9f7685314..7d424b9bf6f6 100644 --- a/src/quic/bindingdata.h +++ b/src/quic/bindingdata.h @@ -24,6 +24,7 @@ class Endpoint; class Packet; class Session; class SessionManager; +struct QuicAllocState; // ============================================================================ @@ -281,15 +282,12 @@ class BindingData final // NgLibMemoryManager — the base class provides CheckAllocatedSize, // IncreaseAllocatedSize, DecreaseAllocatedSize, and StopTrackingMemory. - // Actual allocations go through the thread-local allocators below. + // Actual allocations go through the allocators below. void CheckAllocatedSize(size_t previous_size) const; void IncreaseAllocatedSize(size_t size); void DecreaseAllocatedSize(size_t size); - // Thread-local allocators that outlive BindingData destruction. - // Both ngtcp2 and nghttp3 store the allocator pointer inside every - // object they allocate; some of those objects (e.g., nghttp3 rcbufs - // backing V8 external strings) can be freed after BindingData is gone. + // The allocators can outlive the BindingData; see QuicAllocState. ngtcp2_mem* ngtcp2_allocator(); nghttp3_mem* nghttp3_allocator(); @@ -384,6 +382,8 @@ class BindingData final ArenaPtr endpoint_state_arena_{nullptr, +[](void*) {}}; ArenaPtr endpoint_stats_arena_{nullptr, +[](void*) {}}; + QuicAllocState* alloc_state_; + // Deferred send flush state. The CheckWrapHandle fires immediately after // the I/O poll phase in the same event loop tick, allowing batched // receive processing: all packets are read during poll, then diff --git a/src/quic/data.cc b/src/quic/data.cc index 9599adec62f8..f266caf68132 100644 --- a/src/quic/data.cc +++ b/src/quic/data.cc @@ -33,6 +33,7 @@ using v8::Undefined; using v8::Value; namespace quic { +// NOLINTNEXTLINE(runtime/thread_local) thread_local int DebugIndentScope::indent_ = 0; Path::Path(const SocketAddress& local, const SocketAddress& remote) { diff --git a/src/quic/defs.h b/src/quic/defs.h index 45b4c77d1584..2f249b4ee469 100644 --- a/src/quic/defs.h +++ b/src/quic/defs.h @@ -395,6 +395,7 @@ class DebugIndentScope final { } private: + // NOLINTNEXTLINE(runtime/thread_local) static thread_local int indent_; }; diff --git a/test/cctest/test_environment_shared_isolate.cc b/test/cctest/test_environment_shared_isolate.cc new file mode 100644 index 000000000000..def48ab88f3b --- /dev/null +++ b/test/cctest/test_environment_shared_isolate.cc @@ -0,0 +1,524 @@ +// Several node::Environments on one v8::Isolate that the embedder created, +// registered with the platform and gave a CppHeap, each in its own +// embedder-created v8::Context, driven by one event loop and freed one at a +// time while the others keep running. doc/api/embedding.md allows this and +// embedders that host Node.js inside an existing JS runtime depend on it. + +#include "cppgc/allocation.h" +#include "cppgc/garbage-collected.h" +#include "env-inl.h" +#include "node_test_fixture.h" +#include "v8-cppgc.h" +#if HAVE_OPENSSL +#include "crypto/crypto_context.h" +#endif +#if HAVE_QUIC +#include "node_realm-inl.h" +#include "quic/bindingdata.h" +#endif + +#include +#include + +using node::Environment; +using node::IsolateData; +using v8::Context; +using v8::HandleScope; +using v8::Isolate; +using v8::Local; +using v8::Value; +namespace EnvironmentFlags = node::EnvironmentFlags; + +namespace { + +class EmbedderObject final : public v8::Object::Wrappable { + public: + static int alive; + EmbedderObject() { alive++; } + ~EmbedderObject() override { alive--; } +}; +int EmbedderObject::alive = 0; + +void SetFlag(void* flag) { + *static_cast(flag) = true; +} + +int cleanup_hook_runs = 0; +void CountCleanupHook(void* arg) { + (*static_cast(arg))++; +} + +enum class IsolateDataMode { + // One IsolateData created with the platform, shared by every Environment. + kShared, + // One IsolateData per Environment, created without a platform and freed + // together with its Environment. + kPerEnvironmentWithoutPlatform, +}; + +struct Instance { + int id; + v8::Global context; + IsolateData* isolate_data = nullptr; + Environment* env = nullptr; + bool exit_handler_called = false; + int exit_code = -1; + node::StopFlags::Flags stop_flags = node::StopFlags::kDoNotTerminateIsolate; +}; + +} // namespace + +class SharedIsolateTest + : public NodeZeroIsolateTestFixture, + public ::testing::WithParamInterface { + protected: + Isolate* isolate_ = nullptr; + v8::CppHeap* cpp_heap_ = nullptr; + IsolateData* shared_isolate_data_ = nullptr; + + void SetUp() override { + NodeZeroIsolateTestFixture::SetUp(); + + Isolate::CreateParams params; + params.array_buffer_allocator = allocator.get(); + params.cpp_heap = + v8::CppHeap::Create(platform.get(), v8::CppHeapCreateParams{{}}) + .release(); + cpp_heap_ = params.cpp_heap; + + isolate_ = Isolate::Allocate(); + CHECK_NOT_NULL(isolate_); + platform->RegisterIsolate(isolate_, ¤t_loop); + Isolate::Initialize(isolate_, params); + node::IsolateSettings settings; + settings.flags |= + node::IsolateSettingsFlags::SHOULD_NOT_SET_PREPARE_STACK_TRACE_CALLBACK; + node::SetIsolateUpForNode(isolate_, settings); + isolate_->Enter(); + + if (GetParam() == IsolateDataMode::kShared) { + HandleScope handle_scope(isolate_); + shared_isolate_data_ = + node::CreateIsolateData(isolate_, ¤t_loop, platform.get()); + CHECK_NOT_NULL(shared_isolate_data_); + } + } + + void TearDown() override { + if (shared_isolate_data_ != nullptr) { + node::FreeIsolateData(shared_isolate_data_); + shared_isolate_data_ = nullptr; + } + EXPECT_EQ(isolate_->GetCppHeap(), cpp_heap_); + platform->DrainTasks(isolate_); + isolate_->Exit(); + bool platform_finished = false; + platform->AddIsolateFinishedCallback(isolate_, SetFlag, &platform_finished); + isolate_->Dispose(); + platform->UnregisterIsolate(isolate_); + while (!platform_finished) uv_run(¤t_loop, UV_RUN_ONCE); + isolate_ = nullptr; + } + + std::unique_ptr CreateInstance(int id, + EnvironmentFlags::Flags flags, + bool throw_after_bootstrap = false) { + auto instance = std::make_unique(); + instance->id = id; + HandleScope handle_scope(isolate_); + Local context = Context::New(isolate_); + CHECK(node::InitializeContext(context).FromJust()); + instance->context.Reset(isolate_, context); + Context::Scope context_scope(context); + + if (GetParam() == IsolateDataMode::kShared) { + instance->isolate_data = shared_isolate_data_; + } else { + instance->isolate_data = + node::CreateIsolateData(isolate_, ¤t_loop, nullptr); + CHECK_NOT_NULL(instance->isolate_data); + } + + std::vector args{"node"}; + if (throw_after_bootstrap) args.push_back("--throw"); + std::vector exec_args; + instance->env = node::CreateEnvironment( + instance->isolate_data, context, args, exec_args, flags); + CHECK_NOT_NULL(instance->env); + + Instance* raw = instance.get(); + if (throw_after_bootstrap) instance->stop_flags = node::StopFlags::kNoFlags; + node::SetProcessExitHandler(instance->env, + [raw](Environment* env, int exit_code) { + raw->exit_handler_called = true; + raw->exit_code = exit_code; + node::Stop(env, raw->stop_flags); + }); + + std::string script = + "const id = " + std::to_string(id) + + ";\n" + "const vm = require('vm');\n" + "const { setInterval, clearInterval, setImmediate } =" + " require('timers');\n" + "require('net');\n" + "globalThis.state = { id, ticks: 0, immediates: 0, events: [] };\n" + "const interval = setInterval(() => {\n" + " state.ticks++;\n" + " new vm.Script('1 + 1');\n" + "}, 1);\n" + "(function again() {\n" + " state.immediates++;\n" + " setImmediate(again);\n" + "})();\n" + "process.on('beforeExit', () => state.events.push('beforeExit'));\n" + "process.on('exit', () => {\n" + " state.events.push('exit');\n" + " clearInterval(interval);\n" + "});\n" + "if (process.argv.includes('--throw')) throw new Error('uncaught');\n" + "return id;\n"; + v8::MaybeLocal result = + node::LoadEnvironment(instance->env, script.c_str()); + if (throw_after_bootstrap) { + CHECK(result.IsEmpty()); + CHECK(instance->exit_handler_called); + } else { + CHECK_EQ(result.ToLocalChecked()->Int32Value(context).FromJust(), id); + } + + node::AddEnvironmentCleanupHook( + isolate_, CountCleanupHook, &cleanup_hook_runs); + return instance; + } + + void FreeInstance(std::unique_ptr instance) { + node::FreeEnvironment(instance->env); + if (GetParam() == IsolateDataMode::kPerEnvironmentWithoutPlatform) { + node::FreeIsolateData(instance->isolate_data); + } + instance->context.Reset(); + } + + Local Evaluate(Instance* instance, const char* source) { + v8::EscapableHandleScope handle_scope(isolate_); + Local context = instance->context.Get(isolate_); + Context::Scope context_scope(context); + Local script = + v8::Script::Compile( + context, v8::String::NewFromUtf8(isolate_, source).ToLocalChecked()) + .ToLocalChecked(); + return handle_scope.Escape(script->Run(context).ToLocalChecked()); + } + + bool TryEvaluate(Instance* instance, const char* source) { + HandleScope handle_scope(isolate_); + Local context = instance->context.Get(isolate_); + Context::Scope context_scope(context); + v8::TryCatch try_catch(isolate_); + Local script = + v8::Script::Compile( + context, v8::String::NewFromUtf8(isolate_, source).ToLocalChecked()) + .ToLocalChecked(); + return !script->Run(context).IsEmpty() && !try_catch.HasTerminated(); + } + + int EvaluateInt(Instance* instance, const char* source) { + HandleScope handle_scope(isolate_); + Local context = instance->context.Get(isolate_); + return Evaluate(instance, source)->Int32Value(context).FromJust(); + } + + std::string EvaluateString(Instance* instance, const char* source) { + HandleScope handle_scope(isolate_); + v8::String::Utf8Value utf8(isolate_, Evaluate(instance, source)); + return *utf8; + } + + // What an embedder's message pump does: a bounded number of loop turns and + // foreground task flushes, never SpinEventLoop() on one Environment. + void PumpLoop(int turns) { + for (int i = 0; i < turns; i++) { + uv_run(¤t_loop, UV_RUN_NOWAIT); + platform->DrainTasks(isolate_); + } + } + + // Runs the loop until every instance's interval timer and immediate chain + // have both run again; a callback lost to a stray termination or exception + // breaks its chain and shows up here. + void PumpUntilAllTicked(const std::vector& instances) { + std::vector ticks, immediates; + for (Instance* instance : instances) { + ticks.push_back(EvaluateInt(instance, "state.ticks")); + immediates.push_back(EvaluateInt(instance, "state.immediates")); + } + for (int turn = 0; turn < 10000; turn++) { + uv_run(¤t_loop, UV_RUN_ONCE); + platform->DrainTasks(isolate_); + bool all_progressed = true; + for (size_t i = 0; i < instances.size(); i++) { + if (EvaluateInt(instances[i], "state.ticks") <= ticks[i] || + EvaluateInt(instances[i], "state.immediates") <= immediates[i]) { + all_progressed = false; + } + } + if (all_progressed) return; + } + FAIL() << "an Environment stopped running its timers or immediates"; + } +}; + +TEST_P(SharedIsolateTest, EnvironmentsComeAndGoWhileSiblingsRun) { + const HandleScope handle_scope(isolate_); + cleanup_hook_runs = 0; + + const auto sibling_flags = static_cast( + EnvironmentFlags::kNoCreateInspector | + EnvironmentFlags::kNoBrowserGlobals | + EnvironmentFlags::kNoRegisterESMLoader | + EnvironmentFlags::kNoGlobalSearchPaths); + + std::unique_ptr owner = + CreateInstance(0, EnvironmentFlags::kDefaultFlags); + std::unique_ptr second = CreateInstance(1, sibling_flags); + std::unique_ptr third = CreateInstance(2, sibling_flags); + EXPECT_EQ(isolate_->GetCppHeap(), cpp_heap_); + + PumpUntilAllTicked({owner.get(), second.get(), third.get()}); + + // The embedder's own cppgc objects live on the same heap as Node's. + v8::Global embedder_holder; + { + HandleScope inner(isolate_); + Local context = second->context.Get(isolate_); + Context::Scope context_scope(context); + Local holder = v8::FunctionTemplate::New(isolate_) + ->InstanceTemplate() + ->NewInstance(context) + .ToLocalChecked(); + v8::Object::Wrap( + isolate_, + holder, + cppgc::MakeGarbageCollected( + isolate_->GetCppHeap()->GetAllocationHandle())); + embedder_holder.Reset(isolate_, holder); + } + isolate_->LowMemoryNotification(); + EXPECT_EQ(EmbedderObject::alive, 1); + +#if HAVE_INSPECTOR + EXPECT_EQ(EvaluateInt( + owner.get(), + "(() => {\n" + " const { Session } = process.getBuiltinModule('inspector');\n" + " const session = new Session();\n" + " session.connect();\n" + " let value = -1;\n" + " session.post('Runtime.evaluate', { expression: 'state.id + " + "42' },\n" + " (err, res) => { value = err ? -2 : " + "res.result.value; });\n" + " console.time('session'); console.timeEnd('session');\n" + " session.disconnect();\n" + " return value;\n" + "})()"), + 42); + // A sibling without an inspector of its own gets an exception, not an abort. + EXPECT_EQ( + EvaluateString( + second.get(), + "(() => {\n" + " try {\n" + " const { Session } = process.getBuiltinModule('inspector');\n" + " const session = new Session();\n" + " session.connect();\n" + " session.disconnect();\n" + " return 'connected';\n" + " } catch (e) { return String(e.code); }\n" + "})()"), + "ERR_INSPECTOR_NOT_AVAILABLE"); +#endif // HAVE_INSPECTOR + + if (GetParam() == IsolateDataMode::kShared) { + // Workers need a platform; their exit must not disturb the siblings. + EXPECT_EQ( + EvaluateInt(third.get(), + "(() => {\n" + " const { Worker } =" + " process.getBuiltinModule('worker_threads');\n" + " state.workerExit = -1;\n" + " new Worker('process.exit(7)', { eval: true })\n" + " .on('exit', (code) => { state.workerExit = code; });\n" + " return 0;\n" + "})()"), + 0); + for (int turn = 0; turn < 10000; turn++) { + PumpLoop(1); + if (EvaluateInt(third.get(), "state.workerExit") == 7) break; + } + EXPECT_EQ(EvaluateInt(third.get(), "state.workerExit"), 7); + } + + PumpUntilAllTicked({owner.get(), second.get(), third.get()}); + + // Free one Environment while its siblings have timers and immediates due. + const int owner_ticks = EvaluateInt(owner.get(), "state.ticks"); + FreeInstance(std::move(second)); + EXPECT_EQ(cleanup_hook_runs, 1); + EXPECT_FALSE(owner->exit_handler_called); + EXPECT_FALSE(third->exit_handler_called); + PumpUntilAllTicked({owner.get(), third.get()}); + EXPECT_GT(EvaluateInt(owner.get(), "state.ticks"), owner_ticks); + + // The embedder's object outlives the Environment whose context wrapped it. + isolate_->LowMemoryNotification(); + EXPECT_EQ(EmbedderObject::alive, 1); + embedder_holder.Reset(); + + // An uncaught exception whose exit handler calls Stop() without + // kDoNotTerminateIsolate, then free: the termination it requests must not + // leak into the sibling's next script. + third->stop_flags = node::StopFlags::kNoFlags; + EXPECT_EQ(EvaluateInt(third.get(), + "process.getBuiltinModule('timers').setImmediate(" + "() => { throw new Error('uncaught'); }), 0"), + 0); + for (int turn = 0; turn < 100 && !third->exit_handler_called; turn++) { + PumpLoop(1); + } + EXPECT_TRUE(third->exit_handler_called); + EXPECT_EQ(third->exit_code, 1); + FreeInstance(std::move(third)); + EXPECT_EQ(cleanup_hook_runs, 2); + EXPECT_TRUE(TryEvaluate(owner.get(), "state.ticks")); + PumpUntilAllTicked({owner.get()}); + EXPECT_FALSE(owner->exit_handler_called); + + // The same when the exception is thrown by the bootstrap script itself and + // no JavaScript of that Environment runs after its exit handler. + std::unique_ptr failed = CreateInstance(4, sibling_flags, true); + EXPECT_EQ(failed->exit_code, 1); + FreeInstance(std::move(failed)); + EXPECT_EQ(cleanup_hook_runs, 3); + EXPECT_TRUE(TryEvaluate(owner.get(), "state.ticks")); + PumpUntilAllTicked({owner.get()}); + + // A new sibling can join after others left, here a second one that owns an + // inspector and debug signal handler of its own. + std::unique_ptr late = + CreateInstance(3, EnvironmentFlags::kDefaultFlags); + PumpUntilAllTicked({owner.get(), late.get()}); + + // beforeExit / exit are per Environment. + { + HandleScope inner(isolate_); + Context::Scope context_scope(late->context.Get(isolate_)); + node::EmitProcessBeforeExit(late->env).Check(); + } + EXPECT_EQ(EvaluateString(late.get(), "state.events.join()"), "beforeExit"); + EXPECT_EQ(EvaluateString(owner.get(), "state.events.join()"), ""); + { + HandleScope inner(isolate_); + Context::Scope context_scope(late->context.Get(isolate_)); + EXPECT_EQ(node::EmitProcessExit(late->env).FromJust(), 0); + } + EXPECT_EQ(EvaluateString(late.get(), "state.events.join()"), + "beforeExit,exit"); + EXPECT_EQ(EvaluateString(owner.get(), "state.events.join()"), ""); + node::Stop(late->env, node::StopFlags::kDoNotTerminateIsolate); + PumpUntilAllTicked({owner.get()}); + EXPECT_EQ(EvaluateInt(late.get(), "state.events.length"), 2); + FreeInstance(std::move(late)); + EXPECT_EQ(cleanup_hook_runs, 4); + + PumpUntilAllTicked({owner.get()}); + node::Stop(owner->env, node::StopFlags::kDoNotTerminateIsolate); + FreeInstance(std::move(owner)); + EXPECT_EQ(cleanup_hook_runs, 5); + + // Nothing the Environments left behind keeps the shared loop busy. + PumpLoop(2); + EXPECT_EQ(uv_loop_alive(¤t_loop), 0); +} + +TEST_P(SharedIsolateTest, FreeIsolateDataBeforeItsEnvironmentAsserts) { + GTEST_FLAG_SET(death_test_style, "threadsafe"); + const HandleScope handle_scope(isolate_); + std::unique_ptr instance = + CreateInstance(0, EnvironmentFlags::kNoCreateInspector); + IsolateData* isolate_data = instance->isolate_data; + EXPECT_DEATH_IF_SUPPORTED(node::FreeIsolateData(isolate_data), + "environment_count_"); + node::Stop(instance->env, node::StopFlags::kDoNotTerminateIsolate); + FreeInstance(std::move(instance)); +} + +#if HAVE_OPENSSL +TEST_P(SharedIsolateTest, RootCertStoreIsPerEnvironment) { + const HandleScope handle_scope(isolate_); + std::unique_ptr first = + CreateInstance(0, EnvironmentFlags::kNoCreateInspector); + std::unique_ptr second = + CreateInstance(1, EnvironmentFlags::kNoCreateInspector); + auto store_size = [](Instance* instance) { + return sk_X509_OBJECT_num(X509_STORE_get0_objects( + node::crypto::GetOrCreateRootCertStore(instance->env))); + }; + const int default_size = store_size(second.get()); + EXPECT_GT(default_size, 0); + + Evaluate(first.get(), + "process.getBuiltinModule('tls').setDefaultCACertificates([])"); + EXPECT_EQ(store_size(first.get()), 0); + EXPECT_EQ(store_size(second.get()), default_size); + + FreeInstance(std::move(first)); + EXPECT_EQ(store_size(second.get()), default_size); + FreeInstance(std::move(second)); +} +#endif // HAVE_OPENSSL + +#if HAVE_QUIC +TEST_P(SharedIsolateTest, QuicAllocatorIsPerEnvironment) { + const HandleScope handle_scope(isolate_); + std::unique_ptr first = + CreateInstance(0, EnvironmentFlags::kNoCreateInspector); + std::unique_ptr second = + CreateInstance(1, EnvironmentFlags::kNoCreateInspector); + auto allocator = [this](Instance* instance) { + HandleScope inner(isolate_); + Local context = instance->context.Get(isolate_); + Context::Scope context_scope(context); + Local name = v8::String::NewFromUtf8Literal(isolate_, "quic"); + instance->env->principal_realm() + ->internal_binding_loader() + ->Call(context, v8::Undefined(isolate_), 1, &name) + .ToLocalChecked(); + return node::quic::BindingData::Get(instance->env).ngtcp2_allocator(); + }; + + ngtcp2_mem* first_mem = allocator(first.get()); + void* first_ptr = first_mem->malloc(64, first_mem->user_data); + ngtcp2_mem* second_mem = allocator(second.get()); + void* second_ptr = second_mem->malloc(16, second_mem->user_data); + EXPECT_NE(first_mem, second_mem); + first_mem->free(first_ptr, first_mem->user_data); + + FreeInstance(std::move(second)); + second_ptr = second_mem->realloc(second_ptr, 32, second_mem->user_data); + second_mem->free(second_ptr, second_mem->user_data); + FreeInstance(std::move(first)); +} +#endif // HAVE_QUIC + +INSTANTIATE_TEST_SUITE_P( + EnvironmentTest, + SharedIsolateTest, + ::testing::Values(IsolateDataMode::kShared, + IsolateDataMode::kPerEnvironmentWithoutPlatform), + [](const ::testing::TestParamInfo& info) { + return info.param == IsolateDataMode::kShared + ? "SharedIsolateData" + : "IsolateDataPerEnvironmentWithoutPlatform"; + }); diff --git a/tools/cpplint.py b/tools/cpplint.py index 464d95b8824f..1e26e3ded396 100755 --- a/tools/cpplint.py +++ b/tools/cpplint.py @@ -350,6 +350,7 @@ "runtime/printf_format", "runtime/references", "runtime/string", + "runtime/thread_local", "runtime/threadsafe_fn", "runtime/vlog", "runtime/v8_persistent", @@ -7446,6 +7447,25 @@ def CheckStringValueUsage(filename, lines, error): 'Use node::TwoByteValue instead.') +def CheckThreadLocalUsage(filename, lines, error): + """Logs an error if thread_local is used in src/. + Args: + filename: The name of the current file. + lines: An array of strings, each representing a line of the file. + error: The function to call with any errors found. + """ + if not (filename.startswith('src/') or filename.startswith('src\\')): + return + + for linenum, line in enumerate(lines): + if re.search(r'\bthread_local\b', line.split('//', 1)[0]): + error(filename, linenum, 'runtime/thread_local', 5, + 'Several Environments can share a thread, so keep state that ' + 'belongs to one on the Environment or its BindingData. Mark ' + 'intentionally per-thread state with ' + 'NOLINTNEXTLINE(runtime/thread_local).') + + def ProcessLine( filename, file_extension, @@ -7609,6 +7629,8 @@ def ProcessFileData(filename, file_extension, lines, error, extra_check_function CheckStringValueUsage(filename, lines, error) + CheckThreadLocalUsage(filename, lines, error) + def ProcessConfigOverrides(filename): """Loads the configuration files and processes the config overrides.