From 78d79a4b2a0d0c07e838d3e7cec6dcc6a9ca5cb2 Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Sat, 26 Sep 2026 10:50:59 +0000 Subject: [PATCH 1/4] crypto: keep the root cert store per Environment The root cert store and the certificates set through tls.setDefaultCACertificates() were thread_local, with a cleanup hook on whichever Environment used TLS first. When several Environments share a thread, setting the default CA certificates in one of them replaced the trusted CAs of the others. Keep both on the Environment, next to its other OpenSSL state. Signed-off-by: Shelley Vohr --- src/crypto/crypto_context.cc | 74 +++++++++---------- src/env.cc | 1 + src/env.h | 8 ++ .../cctest/test_environment_shared_isolate.cc | 34 +++++++++ 4 files changed, 76 insertions(+), 41 deletions(-) diff --git a/src/crypto/crypto_context.cc b/src/crypto/crypto_context.cc index 823d87e7f204..d314d15cc542 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,10 @@ 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 X509Set* certs_from_users = + env != nullptr ? GetRootCertStore(env)->certs_from_users.get() : nullptr; + if (certs_from_users != nullptr) { + for (const auto& cert : *certs_from_users) { CHECK_EQ(1, X509_STORE_add_cert(store, cert.get())); } return store; @@ -1230,12 +1222,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 +1239,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 +1256,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 +1267,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 e96b6e6bb129..f745f9ef11d7 100644 --- a/src/env.cc +++ b/src/env.cc @@ -1291,6 +1291,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 diff --git a/src/env.h b/src/env.h index 75f3a66eda7b..4aadd63536e8 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; } @@ -1220,6 +1227,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/test/cctest/test_environment_shared_isolate.cc b/test/cctest/test_environment_shared_isolate.cc index e8787d45c103..be9638211bf6 100644 --- a/test/cctest/test_environment_shared_isolate.cc +++ b/test/cctest/test_environment_shared_isolate.cc @@ -6,8 +6,12 @@ #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 #include #include @@ -446,6 +450,36 @@ TEST_P(SharedIsolateTest, FreeIsolateDataBeforeItsEnvironmentAsserts) { 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))); + }; + auto set_default_ca_count = [this](Instance* instance, int count) { + std::string source = + "const tls = process.getBuiltinModule('tls');" + "tls.setDefaultCACertificates(tls.rootCertificates.slice(0, " + + std::to_string(count) + "))"; + Evaluate(instance, source.c_str()); + }; + + set_default_ca_count(first.get(), 1); + set_default_ca_count(second.get(), 2); + EXPECT_EQ(store_size(first.get()), 1); + EXPECT_EQ(store_size(second.get()), 2); + + FreeInstance(std::move(first)); + EXPECT_EQ(store_size(second.get()), 2); + FreeInstance(std::move(second)); +} +#endif // HAVE_OPENSSL + INSTANTIATE_TEST_SUITE_P( EnvironmentTest, SharedIsolateTest, From c97c54ec734b8c6b0c746d5c80f6098dd67f048f Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Sat, 26 Sep 2026 10:50:59 +0000 Subject: [PATCH 2/4] quic: give each BindingData its own allocator state The ngtcp2 and nghttp3 allocators shared one thread_local state whose BindingData pointer was set by the last BindingData that handed out an allocator. With several Environments on a thread, memory allocated for a session in one Environment was accounted against another's BindingData and failed a CHECK when freed. Give each BindingData its own heap-allocated state. nghttp3 buffers backing external strings can be freed after the BindingData is gone, so the state counts live allocations and is deleted once the BindingData has been destroyed and the last of them is freed. Signed-off-by: Shelley Vohr --- src/quic/README.md | 12 ++-- src/quic/bindingdata.cc | 63 +++++++++---------- src/quic/bindingdata.h | 10 +-- .../cctest/test_environment_shared_isolate.cc | 41 ++++++++++++ 4 files changed, 81 insertions(+), 45 deletions(-) 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 391172c5979e..e49da50f3f03 100644 --- a/src/quic/bindingdata.cc +++ b/src/quic/bindingdata.cc @@ -36,33 +36,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); @@ -77,6 +74,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; } @@ -95,6 +96,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) { @@ -103,6 +105,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; } @@ -231,7 +234,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(). @@ -239,27 +243,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 { @@ -348,7 +336,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/test/cctest/test_environment_shared_isolate.cc b/test/cctest/test_environment_shared_isolate.cc index be9638211bf6..3e86063b064c 100644 --- a/test/cctest/test_environment_shared_isolate.cc +++ b/test/cctest/test_environment_shared_isolate.cc @@ -8,10 +8,15 @@ #include "cppgc/garbage-collected.h" #include "env-inl.h" #include "node_test_fixture.h" +#include "quic/guard.h" #include "v8-cppgc.h" #if HAVE_OPENSSL #include "crypto/crypto_context.h" #endif +#ifndef OPENSSL_NO_QUIC +#include "node_realm-inl.h" +#include "quic/bindingdata.h" +#endif #include #include @@ -480,6 +485,42 @@ TEST_P(SharedIsolateTest, RootCertStoreIsPerEnvironment) { } #endif // HAVE_OPENSSL +#ifndef OPENSSL_NO_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* tracked = second_mem->malloc(16, second_mem->user_data); + void* untracked = second_mem->malloc(16, second_mem->user_data); + node::quic::BindingData::Get(second->env).StopTrackingMemory(untracked); + EXPECT_NE(first_mem, second_mem); + first_mem->free(first_ptr, first_mem->user_data); + + FreeInstance(std::move(second)); + second_mem->free(untracked, second_mem->user_data); + tracked = second_mem->realloc(tracked, 32, second_mem->user_data); + second_mem->free(tracked, second_mem->user_data); + FreeInstance(std::move(first)); +} +#endif // OPENSSL_NO_QUIC + INSTANTIATE_TEST_SUITE_P( EnvironmentTest, SharedIsolateTest, From 6939c52d6f899ef09345f05972d27d1a5e502ed9 Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Sat, 26 Sep 2026 10:50:59 +0000 Subject: [PATCH 3/4] tools: lint thread_local in src Several Environments can share a thread, so state in src/ that belongs to one of them cannot be kept in a thread_local. Add a cpplint rule that rejects thread_local in src/ unless the declaration is marked with NOLINTNEXTLINE(runtime/thread_local), and mark the existing uses, which are per-thread by design. Signed-off-by: Shelley Vohr --- src/api/environment.cc | 1 + src/env.cc | 1 + src/node_binding.cc | 2 ++ src/node_debug.cc | 2 ++ src/node_errors.cc | 2 ++ src/node_internals.h | 1 + src/quic/data.cc | 1 + src/quic/defs.h | 1 + tools/cpplint.py | 22 ++++++++++++++++++++++ 9 files changed, 33 insertions(+) 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/env.cc b/src/env.cc index f745f9ef11d7..54dbe82ed3ea 100644 --- a/src/env.cc +++ b/src/env.cc @@ -1473,6 +1473,7 @@ 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() { 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 1c4da8e2d203..bb45d73e9090 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -286,6 +286,7 @@ class InternalCallbackScope { // 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 { diff --git a/src/quic/data.cc b/src/quic/data.cc index fd4c3253432f..f15051da498f 100644 --- a/src/quic/data.cc +++ b/src/quic/data.cc @@ -32,6 +32,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/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. From 94d5ce4cb5c1d43171bbdf6b2f65a72e3d32c044 Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Wed, 30 Sep 2026 12:48:52 +0000 Subject: [PATCH 4/4] fixup! crypto: keep the root cert store per Environment Signed-off-by: Shelley Vohr --- src/env.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/src/env.cc b/src/env.cc index 54dbe82ed3ea..2da45b82f1a2 100644 --- a/src/env.cc +++ b/src/env.cc @@ -1291,7 +1291,6 @@ 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