From 13adffc042cf67f3e8f6b7cbf27268c298ae2ff3 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Tue, 3 Mar 2026 17:55:11 +0100 Subject: [PATCH 1/3] src: make AsyncWrap subclass internal field counts explicit This helps ensure that we already set the correct number of internal fields when creating objects, even if the number of internal fields of e.g. AsyncWrap changes over time. --- src/async_wrap-inl.h | 9 +++++++++ src/async_wrap.h | 7 +++++++ src/base_object.cc | 9 +++++---- src/base_object.h | 4 ++-- src/cares_wrap.cc | 12 +++--------- src/crypto/crypto_keys.cc | 2 +- src/crypto/crypto_sig.cc | 4 ++-- src/crypto/crypto_tls.cc | 4 ++-- src/crypto/crypto_tls.h | 5 +++++ src/crypto/crypto_util.h | 2 +- src/crypto/crypto_x509.cc | 2 +- src/heap_utils.cc | 7 ++++++- src/histogram.cc | 4 ++-- src/histogram.h | 12 +++++++++++- src/js_stream.cc | 3 +-- src/js_stream.h | 5 +++++ src/node_blob.cc | 5 ++--- src/node_file.h | 3 ++- src/node_http2.cc | 4 ++-- src/node_http2.h | 5 +++++ src/node_postmortem_metadata.cc | 6 ++++++ src/node_sqlite.cc | 2 +- src/node_v8.cc | 2 +- src/pipe_wrap.cc | 5 ++--- src/quic/logstream.h | 5 +++++ src/stream_base.h | 14 ++++++++++++-- src/stream_wrap.cc | 8 ++++---- src/stream_wrap.h | 5 +++++ src/tcp_wrap.cc | 6 ++---- src/tty_wrap.cc | 2 +- src/udp_wrap.cc | 4 +--- test/cctest/test_node_postmortem_metadata.cc | 18 ++++++++++++++++-- 32 files changed, 130 insertions(+), 55 deletions(-) diff --git a/src/async_wrap-inl.h b/src/async_wrap-inl.h index afef24c7c64e4a..432b22e6c8d185 100644 --- a/src/async_wrap-inl.h +++ b/src/async_wrap-inl.h @@ -89,6 +89,15 @@ inline v8::Local AsyncWrap::GetConstructorTemplate( return GetConstructorTemplate(env->isolate_data()); } +// static +v8::Local AsyncWrap::MakeLazilyInitializedJSTemplate( + Environment* env, int internal_field_count) { + v8::Local t = + BaseObject::MakeLazilyInitializedJSTemplate(env, internal_field_count); + t->Inherit(AsyncWrap::GetConstructorTemplate(env)); + return t; +} + } // namespace node #endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS diff --git a/src/async_wrap.h b/src/async_wrap.h index efb56a61a3e2d3..d0069d10f0f5f1 100644 --- a/src/async_wrap.h +++ b/src/async_wrap.h @@ -118,6 +118,10 @@ class ExternalReferenceRegistry; class AsyncWrap : public BaseObject { public: + enum InternalFields { + kInternalFieldCount = BaseObject::kInternalFieldCount, + }; + enum ProviderType { #define V(PROVIDER) \ PROVIDER_ ## PROVIDER, @@ -229,6 +233,9 @@ class AsyncWrap : public BaseObject { bool IsDoneInitializing() const override; + static inline v8::Local MakeLazilyInitializedJSTemplate( + Environment* env, int internal_field_count = kInternalFieldCount); + private: ProviderType provider_type_ = PROVIDER_NONE; bool init_hook_ran_ = false; diff --git a/src/base_object.cc b/src/base_object.cc index 36e7ee5ffb5250..102832c58c62ee 100644 --- a/src/base_object.cc +++ b/src/base_object.cc @@ -82,15 +82,16 @@ void BaseObject::LazilyInitializedJSTemplateConstructor( } Local BaseObject::MakeLazilyInitializedJSTemplate( - Environment* env) { - return MakeLazilyInitializedJSTemplate(env->isolate_data()); + Environment* env, int internal_field_count) { + return MakeLazilyInitializedJSTemplate(env->isolate_data(), + internal_field_count); } Local BaseObject::MakeLazilyInitializedJSTemplate( - IsolateData* isolate_data) { + IsolateData* isolate_data, int internal_field_count) { Local t = NewFunctionTemplate( isolate_data->isolate(), LazilyInitializedJSTemplateConstructor); - t->InstanceTemplate()->SetInternalFieldCount(BaseObject::kInternalFieldCount); + t->InstanceTemplate()->SetInternalFieldCount(internal_field_count); return t; } diff --git a/src/base_object.h b/src/base_object.h index f838bc01036720..1f7a35497faf15 100644 --- a/src/base_object.h +++ b/src/base_object.h @@ -110,9 +110,9 @@ class BaseObject : public MemoryRetainer { // the `BaseObject*` pointer) and a constructor that initializes that field // to `nullptr`. static v8::Local MakeLazilyInitializedJSTemplate( - IsolateData* isolate); + IsolateData* isolate, int internal_field_count = kInternalFieldCount); static v8::Local MakeLazilyInitializedJSTemplate( - Environment* env); + Environment* env, int internal_field_count = kInternalFieldCount); // Setter/Getter pair for internal fields that can be passed to SetAccessor. template diff --git a/src/cares_wrap.cc b/src/cares_wrap.cc index d224ca300a68be..72f3d06fe07569 100644 --- a/src/cares_wrap.cc +++ b/src/cares_wrap.cc @@ -2318,19 +2318,13 @@ void Initialize(Local target, NODE_DEFINE_CONSTANT(target, DNS_ORDER_IPV4_FIRST); NODE_DEFINE_CONSTANT(target, DNS_ORDER_IPV6_FIRST); - Local aiw = - BaseObject::MakeLazilyInitializedJSTemplate(env); - aiw->Inherit(AsyncWrap::GetConstructorTemplate(env)); + Local aiw = AsyncWrap::MakeLazilyInitializedJSTemplate(env); SetConstructorFunction(context, target, "GetAddrInfoReqWrap", aiw); - Local niw = - BaseObject::MakeLazilyInitializedJSTemplate(env); - niw->Inherit(AsyncWrap::GetConstructorTemplate(env)); + Local niw = AsyncWrap::MakeLazilyInitializedJSTemplate(env); SetConstructorFunction(context, target, "GetNameInfoReqWrap", niw); - Local qrw = - BaseObject::MakeLazilyInitializedJSTemplate(env); - qrw->Inherit(AsyncWrap::GetConstructorTemplate(env)); + Local qrw = AsyncWrap::MakeLazilyInitializedJSTemplate(env); SetConstructorFunction(context, target, "QueryReqWrap", qrw); Local channel_wrap = diff --git a/src/crypto/crypto_keys.cc b/src/crypto/crypto_keys.cc index 2e571fc6ba5564..93a741fb1758a4 100644 --- a/src/crypto/crypto_keys.cc +++ b/src/crypto/crypto_keys.cc @@ -1217,7 +1217,7 @@ void NativeKeyObject::CreateNativeKeyObjectClass( Local t = NewFunctionTemplate(isolate, NativeKeyObject::New); t->InstanceTemplate()->SetInternalFieldCount( - KeyObjectHandle::kInternalFieldCount); + NativeKeyObject::kInternalFieldCount); Local ctor; if (!t->GetFunction(env->context()).ToLocal(&ctor)) diff --git a/src/crypto/crypto_sig.cc b/src/crypto/crypto_sig.cc index 3cf450abafb5f5..b4537efbac4bde 100644 --- a/src/crypto/crypto_sig.cc +++ b/src/crypto/crypto_sig.cc @@ -310,7 +310,7 @@ void Sign::Initialize(Environment* env, Local target) { Isolate* isolate = env->isolate(); Local t = NewFunctionTemplate(isolate, New); - t->InstanceTemplate()->SetInternalFieldCount(SignBase::kInternalFieldCount); + t->InstanceTemplate()->SetInternalFieldCount(Sign::kInternalFieldCount); SetProtoMethod(isolate, t, "init", SignInit); SetProtoMethod(isolate, t, "update", SignUpdate); @@ -437,7 +437,7 @@ void Verify::Initialize(Environment* env, Local target) { Isolate* isolate = env->isolate(); Local t = NewFunctionTemplate(isolate, New); - t->InstanceTemplate()->SetInternalFieldCount(SignBase::kInternalFieldCount); + t->InstanceTemplate()->SetInternalFieldCount(Verify::kInternalFieldCount); SetProtoMethod(isolate, t, "init", VerifyInit); SetProtoMethod(isolate, t, "update", VerifyUpdate); diff --git a/src/crypto/crypto_tls.cc b/src/crypto/crypto_tls.cc index 5c80184ffb5eae..31b7905b72d7f9 100644 --- a/src/crypto/crypto_tls.cc +++ b/src/crypto/crypto_tls.cc @@ -2172,11 +2172,11 @@ void TLSWrap::Initialize( NODE_DEFINE_CONSTANT(target, HAVE_SSL_TRACE); - Local t = BaseObject::MakeLazilyInitializedJSTemplate(env); + Local t = AsyncWrap::MakeLazilyInitializedJSTemplate(env); Local tlsWrapString = FIXED_ONE_BYTE_STRING(env->isolate(), "TLSWrap"); t->SetClassName(tlsWrapString); - t->InstanceTemplate()->SetInternalFieldCount(StreamBase::kInternalFieldCount); + t->InstanceTemplate()->SetInternalFieldCount(TLSWrap::kInternalFieldCount); Local get_write_queue_size = FunctionTemplate::New(env->isolate(), diff --git a/src/crypto/crypto_tls.h b/src/crypto/crypto_tls.h index f02c37182ff189..87063b50bb7455 100644 --- a/src/crypto/crypto_tls.h +++ b/src/crypto/crypto_tls.h @@ -43,6 +43,11 @@ class TLSWrap : public AsyncWrap, public StreamBase, public StreamListener { public: + enum InternalFields { + kInternalFieldCount = std::max(AsyncWrap::kInternalFieldCount, + StreamBase::kInternalFieldCount), + }; + enum class Kind { kClient, kServer diff --git a/src/crypto/crypto_util.h b/src/crypto/crypto_util.h index d2620b40c8bc4b..640aec3e22f311 100644 --- a/src/crypto/crypto_util.h +++ b/src/crypto/crypto_util.h @@ -348,7 +348,7 @@ class CryptoJob : public AsyncWrap, public ThreadPoolWork { v8::Local job = NewFunctionTemplate(isolate, new_fn); job->Inherit(AsyncWrap::GetConstructorTemplate(env)); job->InstanceTemplate()->SetInternalFieldCount( - AsyncWrap::kInternalFieldCount); + CryptoJob::kInternalFieldCount); SetProtoMethod(isolate, job, "run", Run); SetConstructorFunction(context, target, CryptoJobTraits::JobName, job); } diff --git a/src/crypto/crypto_x509.cc b/src/crypto/crypto_x509.cc index 6ac4dd2adb38b6..991c4ca6bfb404 100644 --- a/src/crypto/crypto_x509.cc +++ b/src/crypto/crypto_x509.cc @@ -833,7 +833,7 @@ Local X509Certificate::GetConstructorTemplate( Isolate* isolate = env->isolate(); tmpl = NewFunctionTemplate(isolate, nullptr); tmpl->InstanceTemplate()->SetInternalFieldCount( - BaseObject::kInternalFieldCount); + X509Certificate::kInternalFieldCount); tmpl->SetClassName( FIXED_ONE_BYTE_STRING(env->isolate(), "X509Certificate")); SetProtoMethodNoSideEffect(isolate, tmpl, "subject", Subject); diff --git a/src/heap_utils.cc b/src/heap_utils.cc index b64e43eeccb629..e52685546a7ac2 100644 --- a/src/heap_utils.cc +++ b/src/heap_utils.cc @@ -267,6 +267,11 @@ class HeapSnapshotStream : public AsyncWrap, public StreamBase, public v8::OutputStream { public: + enum InternalFields { + kInternalFieldCount = std::max(AsyncWrap::kInternalFieldCount, + StreamBase::kInternalFieldCount), + }; + HeapSnapshotStream( Environment* env, HeapSnapshotPointer&& snapshot, @@ -401,7 +406,7 @@ BaseObjectPtr CreateHeapSnapshotStream( Local os = FunctionTemplate::New(env->isolate()); os->Inherit(AsyncWrap::GetConstructorTemplate(env)); Local ost = os->InstanceTemplate(); - ost->SetInternalFieldCount(StreamBase::kInternalFieldCount); + ost->SetInternalFieldCount(HeapSnapshotStream::kInternalFieldCount); os->SetClassName( FIXED_ONE_BYTE_STRING(env->isolate(), "HeapSnapshotStream")); StreamBase::AddMethods(env, os); diff --git a/src/histogram.cc b/src/histogram.cc index a0edd77e671f8a..8752a419ec4030 100644 --- a/src/histogram.cc +++ b/src/histogram.cc @@ -272,7 +272,7 @@ Local HistogramBase::GetConstructorTemplate( Local classname = FIXED_ONE_BYTE_STRING(isolate, "Histogram"); tmpl->SetClassName(classname); auto instance = tmpl->InstanceTemplate(); - instance->SetInternalFieldCount(HistogramImpl::kInternalFieldCount); + instance->SetInternalFieldCount(HistogramBase::kInternalFieldCount); SetFastMethod(isolate, instance, "record", Record, &fast_record_); SetFastMethod( isolate, instance, "recordDelta", RecordDelta, &fast_record_delta_); @@ -328,7 +328,7 @@ Local IntervalHistogram::GetConstructorTemplate( tmpl->Inherit(HandleWrap::GetConstructorTemplate(env)); tmpl->SetClassName(FIXED_ONE_BYTE_STRING(isolate, "Histogram")); auto instance = tmpl->InstanceTemplate(); - instance->SetInternalFieldCount(HistogramImpl::kInternalFieldCount); + instance->SetInternalFieldCount(IntervalHistogram::kInternalFieldCount); HistogramImpl::AddMethods(isolate, tmpl); SetFastMethod(isolate, instance, "start", Start, &fast_start_); SetFastMethod(isolate, instance, "stop", Stop, &fast_stop_); diff --git a/src/histogram.h b/src/histogram.h index 29303bd16648da..31c6564b9b1f12 100644 --- a/src/histogram.h +++ b/src/histogram.h @@ -71,7 +71,7 @@ class HistogramImpl { public: enum InternalFields { kSlot = BaseObject::kSlot, - kImplField = BaseObject::kInternalFieldCount, + kImplField = HandleWrap::kInternalFieldCount, kInternalFieldCount }; @@ -133,6 +133,11 @@ class HistogramImpl { class HistogramBase final : public BaseObject, public HistogramImpl { public: + enum InternalFields { + kInternalFieldCount = std::max( + BaseObject::kInternalFieldCount, HistogramImpl::kInternalFieldCount), + }; + static v8::Local GetConstructorTemplate( IsolateData* isolate_data); static void Initialize(IsolateData* isolate_data, @@ -203,6 +208,11 @@ class HistogramBase final : public BaseObject, public HistogramImpl { class IntervalHistogram final : public HandleWrap, public HistogramImpl { public: + enum InternalFields { + kInternalFieldCount = std::max( + HandleWrap::kInternalFieldCount, HistogramImpl::kInternalFieldCount), + }; + enum class StartFlags { NONE, RESET diff --git a/src/js_stream.cc b/src/js_stream.cc index cf04e5ef757593..0d6d6a7ac80afd 100644 --- a/src/js_stream.cc +++ b/src/js_stream.cc @@ -210,8 +210,7 @@ void JSStream::Initialize(Local target, Isolate* isolate = env->isolate(); Local t = NewFunctionTemplate(isolate, New); - t->InstanceTemplate() - ->SetInternalFieldCount(StreamBase::kInternalFieldCount); + t->InstanceTemplate()->SetInternalFieldCount(JSStream::kInternalFieldCount); t->Inherit(AsyncWrap::GetConstructorTemplate(env)); SetProtoMethod(isolate, t, "finishWrite", Finish); diff --git a/src/js_stream.h b/src/js_stream.h index d96b3d0062dc1f..b8c88fd812dcb7 100644 --- a/src/js_stream.h +++ b/src/js_stream.h @@ -12,6 +12,11 @@ class Environment; class JSStream : public AsyncWrap, public StreamBase { public: + enum InternalFields { + kInternalFieldCount = std::max(AsyncWrap::kInternalFieldCount, + StreamBase::kInternalFieldCount), + }; + static void Initialize(v8::Local target, v8::Local unused, v8::Local context, diff --git a/src/node_blob.cc b/src/node_blob.cc index 417cd8cbd307b9..6371aad07beb85 100644 --- a/src/node_blob.cc +++ b/src/node_blob.cc @@ -155,8 +155,7 @@ Local Blob::GetConstructorTemplate(Environment* env) { if (tmpl.IsEmpty()) { Isolate* isolate = env->isolate(); tmpl = NewFunctionTemplate(isolate, nullptr); - tmpl->InstanceTemplate()->SetInternalFieldCount( - BaseObject::kInternalFieldCount); + tmpl->InstanceTemplate()->SetInternalFieldCount(Blob::kInternalFieldCount); tmpl->SetClassName( FIXED_ONE_BYTE_STRING(env->isolate(), "Blob")); SetProtoMethod(isolate, tmpl, "getReader", GetReader); @@ -318,7 +317,7 @@ Local Blob::Reader::GetConstructorTemplate(Environment* env) { Isolate* isolate = env->isolate(); tmpl = NewFunctionTemplate(isolate, nullptr); tmpl->InstanceTemplate()->SetInternalFieldCount( - BaseObject::kInternalFieldCount); + Blob::Reader::kInternalFieldCount); tmpl->SetClassName(FIXED_ONE_BYTE_STRING(env->isolate(), "BlobReader")); SetProtoMethod(env->isolate(), tmpl, "pull", Pull); env->set_blob_reader_constructor_template(tmpl); diff --git a/src/node_file.h b/src/node_file.h index e028562fef0412..2213d590659595 100644 --- a/src/node_file.h +++ b/src/node_file.h @@ -321,7 +321,8 @@ class FileHandleReadWrap final : public ReqWrap { class FileHandle final : public AsyncWrap, public StreamBase { public: enum InternalFields { - kFileHandleBaseField = StreamBase::kInternalFieldCount, + kFileHandleBaseField = std::max(AsyncWrap::kInternalFieldCount, + StreamBase::kInternalFieldCount), kClosingPromiseSlot, kInternalFieldCount }; diff --git a/src/node_http2.cc b/src/node_http2.cc index 72b52ef575c50c..e6e7329dc78216 100644 --- a/src/node_http2.cc +++ b/src/node_http2.cc @@ -3476,7 +3476,7 @@ void Initialize(Local target, Local setting = FunctionTemplate::New(env->isolate()); setting->Inherit(AsyncWrap::GetConstructorTemplate(env)); Local settingt = setting->InstanceTemplate(); - settingt->SetInternalFieldCount(AsyncWrap::kInternalFieldCount); + settingt->SetInternalFieldCount(Http2Settings::kInternalFieldCount); env->set_http2settings_constructor_template(settingt); Local stream = FunctionTemplate::New(env->isolate()); @@ -3492,7 +3492,7 @@ void Initialize(Local target, stream->Inherit(AsyncWrap::GetConstructorTemplate(env)); StreamBase::AddMethods(env, stream); Local streamt = stream->InstanceTemplate(); - streamt->SetInternalFieldCount(StreamBase::kInternalFieldCount); + streamt->SetInternalFieldCount(Http2Stream::kInternalFieldCount); env->set_http2stream_constructor_template(streamt); SetConstructorFunction(context, target, "Http2Stream", stream); diff --git a/src/node_http2.h b/src/node_http2.h index b93c93b91f107c..259b71840b66d4 100644 --- a/src/node_http2.h +++ b/src/node_http2.h @@ -270,6 +270,11 @@ using Http2Header = NgHeader; class Http2Stream : public AsyncWrap, public StreamBase { public: + enum InternalFields { + kInternalFieldCount = std::max(AsyncWrap::kInternalFieldCount, + StreamBase::kInternalFieldCount), + }; + static Http2Stream* New( Http2Session* session, int32_t id, diff --git a/src/node_postmortem_metadata.cc b/src/node_postmortem_metadata.cc index ea7396e54c8107..e40d82c0a5d53f 100644 --- a/src/node_postmortem_metadata.cc +++ b/src/node_postmortem_metadata.cc @@ -38,6 +38,8 @@ extern "C" { int nodedbg_const_ContextEmbedderIndex__kEnvironment__int; int nodedbg_const_BaseObject__kInternalFieldCount__int; +int nodedbg_const_HandleWrap__kInternalFieldCount__int; +int nodedbg_const_ReqWrap__kInternalFieldCount__int; uintptr_t nodedbg_offset_ExternalString__data__uintptr_t; uintptr_t nodedbg_offset_ReqWrap__req_wrap_queue___ListNode_ReqWrapQueue; @@ -54,6 +56,10 @@ int GenDebugSymbols() { ContextEmbedderIndex::kEnvironment; nodedbg_const_BaseObject__kInternalFieldCount__int = BaseObject::kInternalFieldCount; + nodedbg_const_HandleWrap__kInternalFieldCount__int = + HandleWrap::kInternalFieldCount; + nodedbg_const_ReqWrap__kInternalFieldCount__int = + ReqWrap::kInternalFieldCount; nodedbg_offset_ExternalString__data__uintptr_t = NODE_OFF_EXTSTR_DATA; nodedbg_offset_ReqWrap__req_wrap_queue___ListNode_ReqWrapQueue = diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index 98612f39695897..9c3aa6e0b4dc5f 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -3450,7 +3450,7 @@ Local StatementSyncIterator::GetConstructorTemplate( tmpl = NewFunctionTemplate(isolate, IllegalConstructor); tmpl->SetClassName(FIXED_ONE_BYTE_STRING(isolate, "StatementSyncIterator")); tmpl->InstanceTemplate()->SetInternalFieldCount( - StatementSync::kInternalFieldCount); + StatementSyncIterator::kInternalFieldCount); SetProtoMethod(isolate, tmpl, "next", StatementSyncIterator::Next); SetProtoMethod(isolate, tmpl, "return", StatementSyncIterator::Return); env->set_sqlite_statement_sync_iterator_constructor_template(tmpl); diff --git a/src/node_v8.cc b/src/node_v8.cc index 0a0554d01f79af..12972f83ea0f61 100644 --- a/src/node_v8.cc +++ b/src/node_v8.cc @@ -753,7 +753,7 @@ void Initialize(Local target, // GCProfiler Local t = NewFunctionTemplate(env->isolate(), GCProfiler::New); - t->InstanceTemplate()->SetInternalFieldCount(BaseObject::kInternalFieldCount); + t->InstanceTemplate()->SetInternalFieldCount(GCProfiler::kInternalFieldCount); SetProtoMethod(env->isolate(), t, "start", GCProfiler::Start); SetProtoMethod(env->isolate(), t, "stop", GCProfiler::Stop); SetConstructorFunction(context, target, "GCProfiler", t); diff --git a/src/pipe_wrap.cc b/src/pipe_wrap.cc index 7bd15d59031284..770f0847aec59f 100644 --- a/src/pipe_wrap.cc +++ b/src/pipe_wrap.cc @@ -72,7 +72,7 @@ void PipeWrap::Initialize(Local target, Isolate* isolate = env->isolate(); Local t = NewFunctionTemplate(isolate, New); - t->InstanceTemplate()->SetInternalFieldCount(StreamBase::kInternalFieldCount); + t->InstanceTemplate()->SetInternalFieldCount(PipeWrap::kInternalFieldCount); t->Inherit(LibuvStreamWrap::GetConstructorTemplate(env)); @@ -91,8 +91,7 @@ void PipeWrap::Initialize(Local target, env->set_pipe_constructor_template(t); // Create FunctionTemplate for PipeConnectWrap. - auto cwt = BaseObject::MakeLazilyInitializedJSTemplate(env); - cwt->Inherit(AsyncWrap::GetConstructorTemplate(env)); + auto cwt = AsyncWrap::MakeLazilyInitializedJSTemplate(env); SetConstructorFunction(context, target, "PipeConnectWrap", cwt); // Define constants diff --git a/src/quic/logstream.h b/src/quic/logstream.h index 25254e910b9cd8..b8bb1ebaecb8a0 100644 --- a/src/quic/logstream.h +++ b/src/quic/logstream.h @@ -15,6 +15,11 @@ namespace node::quic { // and Keylog diagnostic data (one instance for each). class LogStream final : public AsyncWrap, public StreamBase { public: + enum InternalFields { + kInternalFieldCount = std::max(AsyncWrap::kInternalFieldCount, + StreamBase::kInternalFieldCount), + }; + JS_CONSTRUCTOR(LogStream); static BaseObjectPtr Create(Environment* env); diff --git a/src/stream_base.h b/src/stream_base.h index ccbd769ceaf3c1..be00134eb1fc77 100644 --- a/src/stream_base.h +++ b/src/stream_base.h @@ -37,7 +37,7 @@ class StreamReq { // BaseObject, and the slots are used for the identical purpose. enum InternalFields { kSlot = BaseObject::kSlot, - kStreamReqField = BaseObject::kInternalFieldCount, + kStreamReqField = AsyncWrap::kInternalFieldCount, kInternalFieldCount }; @@ -310,7 +310,7 @@ class StreamBase : public StreamResource { // BaseObject (it's possible for it not to, however). enum InternalFields { kSlot = BaseObject::kSlot, - kStreamBaseField = BaseObject::kInternalFieldCount, + kStreamBaseField = AsyncWrap::kInternalFieldCount, kOnReadFunctionField, kInternalFieldCount }; @@ -440,6 +440,11 @@ class StreamBase : public StreamResource { template class SimpleShutdownWrap : public ShutdownWrap, public OtherBase { public: + enum InternalFields { + kInternalFieldCount = std::max(ShutdownWrap::kInternalFieldCount, + OtherBase::kInternalFieldCount), + }; + SimpleShutdownWrap(StreamBase* stream, v8::Local req_wrap_obj); @@ -457,6 +462,11 @@ class SimpleShutdownWrap : public ShutdownWrap, public OtherBase { template class SimpleWriteWrap : public WriteWrap, public OtherBase { public: + enum InternalFields { + kInternalFieldCount = std::max(WriteWrap::kInternalFieldCount, + OtherBase::kInternalFieldCount), + }; + SimpleWriteWrap(StreamBase* stream, v8::Local req_wrap_obj); diff --git a/src/stream_wrap.cc b/src/stream_wrap.cc index 1508cc097fbf24..b41f6ac7494730 100644 --- a/src/stream_wrap.cc +++ b/src/stream_wrap.cc @@ -72,7 +72,8 @@ void LibuvStreamWrap::Initialize(Local target, Local sw = NewFunctionTemplate(isolate, IsConstructCallCallback); - sw->InstanceTemplate()->SetInternalFieldCount(StreamReq::kInternalFieldCount); + sw->InstanceTemplate()->SetInternalFieldCount( + ShutdownWrap::kInternalFieldCount); // we need to set handle and callback to null, // so that those fields are created and functions @@ -93,8 +94,7 @@ void LibuvStreamWrap::Initialize(Local target, Local ww = FunctionTemplate::New(isolate, IsConstructCallCallback); - ww->InstanceTemplate()->SetInternalFieldCount( - StreamReq::kInternalFieldCount); + ww->InstanceTemplate()->SetInternalFieldCount(WriteWrap::kInternalFieldCount); ww->Inherit(AsyncWrap::GetConstructorTemplate(env)); SetConstructorFunction(context, target, "WriteWrap", ww); env->set_write_wrap_template(ww->InstanceTemplate()); @@ -141,7 +141,7 @@ Local LibuvStreamWrap::GetConstructorTemplate( tmpl->SetClassName(FIXED_ONE_BYTE_STRING(isolate, "LibuvStreamWrap")); tmpl->Inherit(HandleWrap::GetConstructorTemplate(env)); tmpl->InstanceTemplate()->SetInternalFieldCount( - StreamBase::kInternalFieldCount); + LibuvStreamWrap::kInternalFieldCount); Local get_write_queue_size = FunctionTemplate::New(isolate, GetWriteQueueSize, diff --git a/src/stream_wrap.h b/src/stream_wrap.h index 85a675d04f22f0..93db2a9e686051 100644 --- a/src/stream_wrap.h +++ b/src/stream_wrap.h @@ -35,6 +35,11 @@ class ExternalReferenceRegistry; class LibuvStreamWrap : public HandleWrap, public StreamBase { public: + enum InternalFields { + kInternalFieldCount = std::max(HandleWrap::kInternalFieldCount, + StreamBase::kInternalFieldCount), + }; + static void Initialize(v8::Local target, v8::Local unused, v8::Local context, diff --git a/src/tcp_wrap.cc b/src/tcp_wrap.cc index df9e9493643641..55d1d8516a8cd2 100644 --- a/src/tcp_wrap.cc +++ b/src/tcp_wrap.cc @@ -88,7 +88,7 @@ void TCPWrap::Initialize(Local target, Isolate* isolate = env->isolate(); Local t = NewFunctionTemplate(isolate, New); - t->InstanceTemplate()->SetInternalFieldCount(StreamBase::kInternalFieldCount); + t->InstanceTemplate()->SetInternalFieldCount(TCPWrap::kInternalFieldCount); // Init properties t->InstanceTemplate()->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "reading"), @@ -126,9 +126,7 @@ void TCPWrap::Initialize(Local target, env->set_tcp_constructor_template(t); // Create FunctionTemplate for TCPConnectWrap. - Local cwt = - BaseObject::MakeLazilyInitializedJSTemplate(env); - cwt->Inherit(AsyncWrap::GetConstructorTemplate(env)); + Local cwt = AsyncWrap::MakeLazilyInitializedJSTemplate(env); SetConstructorFunction(context, target, "TCPConnectWrap", cwt); // Define constants diff --git a/src/tty_wrap.cc b/src/tty_wrap.cc index 37c58cf0cb7adc..3086e859a54e61 100644 --- a/src/tty_wrap.cc +++ b/src/tty_wrap.cc @@ -60,7 +60,7 @@ void TTYWrap::Initialize(Local target, Local t = NewFunctionTemplate(isolate, New); t->SetClassName(ttyString); - t->InstanceTemplate()->SetInternalFieldCount(StreamBase::kInternalFieldCount); + t->InstanceTemplate()->SetInternalFieldCount(TTYWrap::kInternalFieldCount); t->Inherit(LibuvStreamWrap::GetConstructorTemplate(env)); SetProtoMethodNoSideEffect( diff --git a/src/udp_wrap.cc b/src/udp_wrap.cc index 2101a208917e8e..b5e732dd6f1cb4 100644 --- a/src/udp_wrap.cc +++ b/src/udp_wrap.cc @@ -226,9 +226,7 @@ void UDPWrap::Initialize(Local target, env->set_udp_constructor_function(t->GetFunction(context).ToLocalChecked()); // Create FunctionTemplate for SendWrap - Local swt = - BaseObject::MakeLazilyInitializedJSTemplate(env); - swt->Inherit(AsyncWrap::GetConstructorTemplate(env)); + Local swt = AsyncWrap::MakeLazilyInitializedJSTemplate(env); SetConstructorFunction(context, target, "SendWrap", swt); Local constants = Object::New(isolate); diff --git a/test/cctest/test_node_postmortem_metadata.cc b/test/cctest/test_node_postmortem_metadata.cc index a6f7b9f3d0e735..fc147195bb17cd 100644 --- a/test/cctest/test_node_postmortem_metadata.cc +++ b/test/cctest/test_node_postmortem_metadata.cc @@ -16,6 +16,8 @@ extern uintptr_t extern int debug_symbols_generated; extern int nodedbg_const_ContextEmbedderIndex__kEnvironment__int; extern int nodedbg_const_BaseObject__kInternalFieldCount__int; +extern int nodedbg_const_HandleWrap__kInternalFieldCount__int; +extern int nodedbg_const_ReqWrap__kInternalFieldCount__int; extern uintptr_t nodedbg_offset_Environment_HandleWrapQueue__head___ListNode_HandleWrap; extern uintptr_t @@ -76,6 +78,18 @@ TEST_F(DebugSymbolsTest, BaseObjectkInternalFieldCount) { kInternalFieldCount); } +TEST_F(DebugSymbolsTest, HandleWrapInternalFieldCount) { + int kInternalFieldCount = node::HandleWrap::kInternalFieldCount; + EXPECT_EQ(nodedbg_const_HandleWrap__kInternalFieldCount__int, + kInternalFieldCount); +} + +TEST_F(DebugSymbolsTest, ReqWrapInternalFieldCount) { + int kInternalFieldCount = node::ReqWrap::kInternalFieldCount; + EXPECT_EQ(nodedbg_const_ReqWrap__kInternalFieldCount__int, + kInternalFieldCount); +} + TEST_F(DebugSymbolsTest, ExternalStringDataOffset) { EXPECT_EQ(nodedbg_offset_ExternalString__data__uintptr_t, NODE_OFF_EXTSTR_DATA); @@ -149,7 +163,7 @@ TEST_F(DebugSymbolsTest, HandleWrapList) { auto obj_template = v8::FunctionTemplate::New(isolate_); obj_template->InstanceTemplate()->SetInternalFieldCount( - nodedbg_const_BaseObject__kInternalFieldCount__int); + nodedbg_const_HandleWrap__kInternalFieldCount__int); v8::Local object = obj_template->GetFunction(env.context()) .ToLocalChecked() @@ -182,7 +196,7 @@ TEST_F(DebugSymbolsTest, ReqWrapList) { auto obj_template = v8::FunctionTemplate::New(isolate_); obj_template->InstanceTemplate()->SetInternalFieldCount( - nodedbg_const_BaseObject__kInternalFieldCount__int); + nodedbg_const_ReqWrap__kInternalFieldCount__int); v8::Local object = obj_template->GetFunction(env.context()) .ToLocalChecked() From 9f06ed52963e28d894e89c579876cf08f67f9003 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Tue, 3 Mar 2026 23:12:18 +0100 Subject: [PATCH 2/3] src: convert context_frame field in AsyncWrap to internal field Using an internal field instead of a `v8::Global<>` removes an unnecessary memory leak footgun. This includes a test that demonstrates the issue, albeit using internal APIs. It's worth noting that if this PR is not accepted, we'd still be missing memory tracking for the `context_frame_` field, and we'd need to add it through our memory tracking API. --- src/async_wrap-inl.h | 20 +++++++- src/async_wrap.cc | 23 +++------ src/async_wrap.h | 6 +-- src/env_properties.h | 1 + ...async-local-storage-weak-asyncwrap-leak.js | 49 +++++++++++++++++++ 5 files changed, 80 insertions(+), 19 deletions(-) create mode 100644 test/parallel/test-async-local-storage-weak-asyncwrap-leak.js diff --git a/src/async_wrap-inl.h b/src/async_wrap-inl.h index 432b22e6c8d185..5dd2b72036ce77 100644 --- a/src/async_wrap-inl.h +++ b/src/async_wrap-inl.h @@ -50,7 +50,25 @@ inline double AsyncWrap::get_trigger_async_id() const { } inline v8::Local AsyncWrap::context_frame() const { - return context_frame_.Get(env()->isolate()); + auto as_data = object()->GetInternalField(kAsyncContextFrame); + DCHECK_IMPLIES(!as_data.IsEmpty(), + as_data->IsValue() || as_data->IsPrivate()); + if (as_data->IsPrivate()) { + DCHECK(as_data.As()->Name()->SameValue( + env()->empty_context_frame_sentinel_symbol()->Name())); + return {}; + } + return as_data.As(); +} + +inline void AsyncWrap::set_context_frame(v8::Local value) { + if (value.IsEmpty()) { + // Empty values are not allowed in internal fields + object()->SetInternalField(kAsyncContextFrame, + env()->empty_context_frame_sentinel_symbol()); + } else { + object()->SetInternalField(kAsyncContextFrame, value); + } } inline v8::MaybeLocal AsyncWrap::MakeCallback( diff --git a/src/async_wrap.cc b/src/async_wrap.cc index 3267a4c8ef8483..654f41437f8c2a 100644 --- a/src/async_wrap.cc +++ b/src/async_wrap.cc @@ -341,7 +341,7 @@ void AsyncWrap::EmitDestroy(bool from_gc) { HandleScope handle_scope(env()->isolate()); USE(object()->Set(env()->context(), env()->resource_symbol(), object())); } - context_frame_.Reset(); + set_context_frame({}); } } @@ -518,9 +518,9 @@ AsyncWrap::AsyncWrap(Environment* env, } AsyncWrap::AsyncWrap(Environment* env, Local object) - : BaseObject(env, object), - context_frame_(env->isolate(), - async_context_frame::current(env->isolate())) {} + : BaseObject(env, object) { + set_context_frame(async_context_frame::current(env->isolate())); +} // This method is necessary to work around one specific problem: // Before the init() hook runs, if there is one, the BaseObject() constructor @@ -623,7 +623,7 @@ void AsyncWrap::AsyncReset(Local resource, double execution_async_id) { EmitTraceAsyncStart(); - context_frame_.Reset(isolate, async_context_frame::current(isolate)); + set_context_frame(async_context_frame::current(isolate)); EmitAsyncInit(env(), resource, env()->async_hooks()->provider_string(provider_type()), @@ -666,7 +666,7 @@ MaybeLocal AsyncWrap::MakeCallback(const Local cb, EmitTraceEventBefore(); #ifdef DEBUG - if (context_frame_.IsEmpty()) { + if (context_frame().IsEmpty()) { ProcessEmitWarning(env(), "MakeCallback() called without context_frame, " "likely use after destroy of AsyncWrap."); @@ -675,15 +675,8 @@ MaybeLocal AsyncWrap::MakeCallback(const Local cb, ProviderType provider = provider_type(); async_context context { get_async_id(), get_trigger_async_id() }; - MaybeLocal ret = - InternalMakeCallback(env(), - object(), - object(), - cb, - argc, - argv, - context, - context_frame_.Get(env()->isolate())); + MaybeLocal ret = InternalMakeCallback( + env(), object(), object(), cb, argc, argv, context, context_frame()); // This is a static call with cached values because the `this` object may // no longer be alive at this point. diff --git a/src/async_wrap.h b/src/async_wrap.h index d0069d10f0f5f1..39e12226963f8c 100644 --- a/src/async_wrap.h +++ b/src/async_wrap.h @@ -119,7 +119,8 @@ class ExternalReferenceRegistry; class AsyncWrap : public BaseObject { public: enum InternalFields { - kInternalFieldCount = BaseObject::kInternalFieldCount, + kAsyncContextFrame = BaseObject::kInternalFieldCount, + kInternalFieldCount, }; enum ProviderType { @@ -199,6 +200,7 @@ class AsyncWrap : public BaseObject { inline double get_trigger_async_id() const; inline v8::Local context_frame() const; + inline void set_context_frame(v8::Local value); void AsyncReset(v8::Local resource, double execution_async_id = kInvalidAsyncId); @@ -242,8 +244,6 @@ class AsyncWrap : public BaseObject { // Because the values may be Reset(), cannot be made const. double async_id_ = kInvalidAsyncId; double trigger_async_id_ = kInvalidAsyncId; - - v8::Global context_frame_; }; } // namespace node diff --git a/src/env_properties.h b/src/env_properties.h index 91e2e06c3c2703..7f0abdac9ca452 100644 --- a/src/env_properties.h +++ b/src/env_properties.h @@ -21,6 +21,7 @@ V(arrow_message_private_symbol, "node:arrowMessage") \ V(contextify_context_private_symbol, "node:contextify:context") \ V(decorated_private_symbol, "node:decorated") \ + V(empty_context_frame_sentinel_symbol, "node:empty_context_frame_sentinel") \ V(transfer_mode_private_symbol, "node:transfer_mode") \ V(host_defined_option_symbol, "node:host_defined_option_symbol") \ V(js_transferable_wrapper_private_symbol, "node:js_transferable_wrapper") \ diff --git a/test/parallel/test-async-local-storage-weak-asyncwrap-leak.js b/test/parallel/test-async-local-storage-weak-asyncwrap-leak.js new file mode 100644 index 00000000000000..baa13f610ed9aa --- /dev/null +++ b/test/parallel/test-async-local-storage-weak-asyncwrap-leak.js @@ -0,0 +1,49 @@ +// Flags: --expose-gc +'use strict'; +const common = require('../common'); +const assert = require('node:assert'); +const zlib = require('node:zlib'); +const v8 = require('node:v8'); +const { AsyncLocalStorage } = require('node:async_hooks'); + +// This test verifies that referencing an AsyncLocalStorage store from +// a weak AsyncWrap does not prevent the store from being garbage collected. +// We use zlib streams as examples of weak AsyncWraps here, but the +// issue is not specific to zlib. + +class Store {} + +let zlibDone = false; + +// Use immediates to ensure no accidental async context propagation occurs +setImmediate(common.mustCall(() => { + const asyncLocalStorage = new AsyncLocalStorage(); + const store = new Store(); + asyncLocalStorage.run(store, common.mustCall(() => { + (async () => { + const zlibStream = zlib.createGzip(); + // (Make sure this test does not break if _handle is renamed + // to something else) + assert.strictEqual(typeof zlibStream._handle, 'object'); + // Create backreference from AsyncWrap to store + store.zlibStream = zlibStream._handle; + // Let the zlib stream finish (not strictly necessary) + zlibStream.end('hello world'); + await zlibStream.toArray(); + zlibDone = true; + })().then(common.mustCall()); + })); +})); + +const finish = common.mustCall(() => { + global.gc(); + // Make sure the ALS instance has been garbage-collected + assert.strictEqual(v8.queryObjects(Store), 0); +}); + +const interval = setInterval(() => { + if (zlibDone) { + clearInterval(interval); + finish(); + } +}, 5); From 8a824880287c5f28d2a9414921e25ad51aaba43e Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Wed, 4 Mar 2026 00:04:37 +0100 Subject: [PATCH 3/3] src: expose async context frame debugging helper to JS This was invaluable for debugging the previous change, so I'm suggesting we add it regardless of it being a debugging-only API. --- src/async_wrap.cc | 12 ++++++++++++ src/async_wrap.h | 2 ++ 2 files changed, 14 insertions(+) diff --git a/src/async_wrap.cc b/src/async_wrap.cc index 654f41437f8c2a..5007fd34414abf 100644 --- a/src/async_wrap.cc +++ b/src/async_wrap.cc @@ -322,6 +322,13 @@ void AsyncWrap::AsyncReset(const FunctionCallbackInfo& args) { wrap->AsyncReset(resource, execution_async_id); } +// Useful for debugging async context propagation. Not intended for public use. +void AsyncWrap::GetAsyncContextFrame(const FunctionCallbackInfo& args) { + AsyncWrap* wrap; + ASSIGN_OR_RETURN_UNWRAP(&wrap, args.This()); + + args.GetReturnValue().Set(wrap->context_frame()); +} void AsyncWrap::GetProviderType(const FunctionCallbackInfo& args) { AsyncWrap* wrap; @@ -372,6 +379,10 @@ Local AsyncWrap::GetConstructorTemplate( FIXED_ONE_BYTE_STRING(isolate_data->isolate(), "AsyncWrap")); SetProtoMethod(isolate, tmpl, "getAsyncId", AsyncWrap::GetAsyncId); SetProtoMethod(isolate, tmpl, "asyncReset", AsyncWrap::AsyncReset); + SetProtoMethod(isolate, + tmpl, + "getAsyncContextFrameForDebuggingOnly", + AsyncWrap::GetAsyncContextFrame); SetProtoMethod( isolate, tmpl, "getProviderType", AsyncWrap::GetProviderType); isolate_data->set_async_wrap_ctor_template(tmpl); @@ -501,6 +512,7 @@ void AsyncWrap::RegisterExternalReferences( registry->Register(RegisterDestroyHook); registry->Register(AsyncWrap::GetAsyncId); registry->Register(AsyncWrap::AsyncReset); + registry->Register(AsyncWrap::GetAsyncContextFrame); registry->Register(AsyncWrap::GetProviderType); } diff --git a/src/async_wrap.h b/src/async_wrap.h index 39e12226963f8c..e4884cb88301d4 100644 --- a/src/async_wrap.h +++ b/src/async_wrap.h @@ -167,6 +167,8 @@ class AsyncWrap : public BaseObject { static void ClearAsyncIdStack( const v8::FunctionCallbackInfo& args); static void AsyncReset(const v8::FunctionCallbackInfo& args); + static void GetAsyncContextFrame( + const v8::FunctionCallbackInfo& args); static void GetProviderType(const v8::FunctionCallbackInfo& args); static void QueueDestroyAsyncId( const v8::FunctionCallbackInfo& args);