diff --git a/bindings/otel-thread-ctx.cc b/bindings/otel-thread-ctx.cc index ddee80cf..faa881c8 100644 --- a/bindings/otel-thread-ctx.cc +++ b/bindings/otel-thread-ctx.cc @@ -23,15 +23,16 @@ // Node.js writer for the OTEP-4947 Thread Local Context Record, adapted for // the Node.js asynchronous context model. The record is wrapped in a JS -// object (CtxWrap) and stored in an AsyncLocalStorage instance; an -// out-of-process reader discovers it by walking the V8 isolate's +// object and stored in an AsyncLocalStorage instance; an out-of-process +// reader discovers it by walking the V8 isolate's // ContinuationPreservedEmbedderData to the AsyncContextFrame (a JS Map), -// looking up the ALS instance as the key, reading the resulting CtxWrap, -// and finally the record it owns. +// looking up the ALS instance as the key, and reading the record pointer +// out of the resulting object's internal field. That field points +// straight at the record. The record is preceded in memory by an instance +// of CtxWrap used for internal bookkeeping. #include "otel-thread-ctx.hh" -#include "defer.hh" #include "internal-field.hh" #include @@ -42,9 +43,9 @@ #include #include +#include #include -#include -#include +#include #include // Single thread-local read from outside the process via TLSDESC. It @@ -62,7 +63,7 @@ // - the (per-isolate) tagged address of the `undefined` singleton // (`undefined_addr`). After looking up the value for our ALS key in // the ACF map, the reader can compare against this to skip the -// JSObject / internal-field-0 dereference when no CtxWrap is +// JSObject / internal-field-0 dereference when no ThreadContext is // currently attached; without it, a reader walking through undefined // would have to rely on structural validation of the bytes at // undefined+wrapped_object_offset to detect the absence. @@ -146,17 +147,13 @@ static_assert(offsetof(OtelThreadCtxRecord, attrs_data_size) == 26, static_assert(offsetof(OtelThreadCtxRecord, attrs_data) == 28, "attrs_data offset"); -struct OtelThreadCtxRecordDeleter { - void operator()(OtelThreadCtxRecord* p) const noexcept { free(p); } -}; -using OwnedRecord = - std::unique_ptr; - // Floor on the attrs_data capacity of a freshly allocated record. Sized so -// the total allocation is one 64-byte cache line — matching the OTEP-4947 +// the record itself is one 64-byte cache line — matching the OTEP-4947 // "frugal writer" guidance ("a frugal writer may aim to keep the entire // record under 64 bytes") — and giving small records some slack so the -// first few appends (if any) can be in-place. +// first few appends (if any) can be in-place. The CtxWrap fields preceding +// the record in the same block are writer bookkeeping the reader never +// sees, so they don't count against that budget. constexpr size_t MIN_INITIAL_CAPACITY = 64 - sizeof(OtelThreadCtxRecord); // Upper bound on the attribute payload. Sized so the total record (28-byte @@ -167,39 +164,29 @@ constexpr size_t MIN_INITIAL_CAPACITY = 64 - sizeof(OtelThreadCtxRecord); // as best-effort. constexpr size_t MAX_ATTRS_DATA_SIZE = 640 - sizeof(OtelThreadCtxRecord); -// Wraps a heap-allocated OtelThreadCtxRecord. Lifetime is managed by V8 -// GC: when no JS code (or AsyncLocalStorage entry) holds a reference, the -// record is freed. -// -// Layout note for the reader: `record_` is private to C++ but its byte -// position within CtxWrap is part of the reader contract. It is the first -// field of the class, at offset zero. `capacity_` and -// `truncated_` sit after `record_` purely for the writer's own -// bookkeeping — the reader never touches them. -// Deliberately not a node::ObjectWrap. That base registers a per-instance -// environment cleanup hook in its constructor and calls -// RemoveEnvironmentCleanupHook from its destructor, which CHECKs that an -// Environment is current: -// -// node[107]: void node::RemoveEnvironmentCleanupHook(...) hooks.cc:142 -// Assertion failed: (env) != nullptr -// 3: otel_thread_ctx_nodejs::CtxWrap::~CtxWrap() +// Wraps an OTEP-4947 record. The record isn't a separate allocation: a +// CtxWrap is one contiguous block whose head is the CtxWrap object itself +// and whose tail — starting at `RECORD_OFFSET`, immediately past the +// object's own fields — holds the record header followed by `capacity_` +// bytes of attrs_data. Lifetime is managed by V8 GC: when no JS code (or +// AsyncLocalStorage entry) holds a reference, the whole block is freed. // -// A CtxWrap is owned by a weak V8 handle, so V8 chooses when it dies, and -// weak callbacks run during isolate teardown with no context entered — -// Environment::GetCurrent(isolate) returns null on `!isolate->InContext()` -// alone — so the CHECK fires and aborts. Reproducible today by creating a few -// thousand ThreadContexts and exiting normally; see the regression test. -// -// Note the CHECK is guarding something real, so this must not be worked -// around by skipping the removal: the Environment may well still be alive, -// and leaving a hook behind whose arg is a freed pointer turns an abort into -// a use-after-free at Drain(). The fix is to never register the per-instance -// hook, and to provide the teardown deletion it was giving us (see -// g_live_ctx_wraps below). +// Layout note for the reader: the holder JSObject's internal field 0 points +// at the record itself, not at the CtxWrap, so a reader that has walked to +// the holder is a single dereference away from the record and never has to +// know that CtxWrap exists at all. C++ code goes the other way with +// FromRecord(), which just subtracts RECORD_OFFSET. +// Deliberately not a node::ObjectWrap, for two independent reasons. First, it +// has a known bug in interaction with GC when numerous instances are created +// and can abort the process during isolate teardown; instances live at +// shutdown are deleted using DrainLive instead. Second, node::ObjectWrap owns +// internal field 0 — its Wrap() stores the ObjectWrap pointer there and its +// Unwrap() reads it back — so deriving from it would force that slot to hold a +// CtxWrap pointer, and every reader would be stuck with the extra hop through +// the wrapper that the layout above exists to avoid. Not deriving from it is +// what frees the slot for the record pointer. class CtxWrap { public: - ~CtxWrap(); static void Init(Local exports); CtxWrap(const CtxWrap&) = delete; @@ -227,28 +214,48 @@ class CtxWrap { std::vector* out, bool* out_truncated); - CtxWrap(OtelThreadCtxRecord* record, size_t capacity, bool truncated); + // Splice `appended` onto the end of the record: in place if it fits in the + // current allocation's slack, otherwise by moving the whole wrap to a larger + // one and repointing `holder`'s internal field at the new record. In that + // case `this` is destroyed before returning, so the caller must not touch it + // afterwards. Returns false if the allocation fails. + bool AppendEncoded(Local holder, + const std::vector& appended); + + explicit CtxWrap(size_t capacity); + ~CtxWrap(); - // Attach to the holder JSObject: store `this` in internal field 0 and take - // a weak handle on the holder, so V8 deletes us once it collects it. + // Allocate one zero-initialised block big enough for the object plus a + // record with `capacity` bytes of attrs_data, and construct the CtxWrap + // at its head. Returns nullptr if the allocation fails. CtxWraps are + // never created with plain `new` — the trailing record wouldn't be there. + static CtxWrap* Create(size_t capacity); + // Destroy and free a CtxWrap obtained from Create(). Runs the destructor + // and then `free`, which is what the `calloc` in Create() pairs with; + // `delete` would reach for `operator delete` instead, and the mismatch is + // undefined behaviour that a replaced global allocator or ASan will + // actually trip over. Deleting `operator delete` below makes reaching for + // it a compile error. + static void Destroy(CtxWrap* self); + static void operator delete(void*) = delete; + + // The record living in the tail of this object's own allocation. + OtelThreadCtxRecord* record(); + // The CtxWrap that precedes the `record`. Inverse of record(). + static CtxWrap* FromRecord(void* record); + + // Attach to the holder JSObject: store our record pointer in internal + // field 0 (the reader's entry point) and take a weak handle on the + // holder, so V8 destroys us once it collects it. void Wrap(Local holder); static CtxWrap* Unwrap(Local holder); static void WeakCallback(const v8::WeakCallbackInfo& data); + static void DrainLive(void* arg); - // The fields are kept in one access section because C++ leaves - // the relative layout of fields in different access controls - // implementation-defined. `record_` must come first — its offset - // within CtxWrap is part of the reader contract (see the - // static_assert below) — and is therefore `public`. The bookkeeping - // fields after it would normally be private, but the access change - // would let a conforming compiler reorder them in front of `record_`; - // exposing them publicly keeps everything in one ordering-stable - // block. Readers never touch them. - public: - OtelThreadCtxRecord* record_; - // attrs_data capacity in bytes of the record_ allocation. The total - // allocation is `sizeof(OtelThreadCtxRecord) + capacity_`. Always - // `record_->attrs_data_size <= capacity_ <= MAX_ATTRS_DATA_SIZE`. + // attrs_data capacity in bytes of the record in this object's tail. The + // total allocation is `RECORD_OFFSET + sizeof(OtelThreadCtxRecord) + + // capacity_`. Always `record()->attrs_data_size <= capacity_ <= + // MAX_ATTRS_DATA_SIZE`. size_t capacity_; // Set to true (once, never cleared) if at any point in this record's // lifetime — during New() or any subsequent Append() — at least one @@ -259,8 +266,8 @@ class CtxWrap { // attribute value, which can execute user JS (e.g. a custom // `toString`) that in turn calls `appendAttributes` on the same // ThreadContext. A reentrant Append would mutate attrs_data_size out - // from under the outer call's `current_used` snapshot, causing the - // outer memcpy to overwrite the reentrant call's bytes and the outer + // from under the outer call's snapshot of it, causing the outer memcpy + // to overwrite the reentrant call's bytes and the outer // attrs_data_size write to shrink the record. We reject the reentrant // call instead. New() doesn't need the guard because a freshly constructed // CtxWrap isn't observable to JS until New() returns. @@ -276,15 +283,33 @@ class CtxWrap { v8::Global handle_; }; -// Pin the offset of `record_` — the field the reader walks to from the -// JSObject's internal field 0. With no base class it is simply the first -// member, so the offset is zero and the published -// `threadlocal.native_wrap_fields_offset` is computed from this. -static_assert(std::is_standard_layout::value, - "CtxWrap must stay standard-layout: the reader contract depends " - "on offsetof(record_) being well-defined"); -static_assert(offsetof(CtxWrap, record_) == 0, - "record_ must be the first field of CtxWrap"); +// Byte offset of the record within a CtxWrap allocation: the record starts +// immediately after the object's own fields. Both record() and FromRecord() +// are defined in terms of it. +constexpr size_t RECORD_OFFSET = sizeof(CtxWrap); +static_assert(RECORD_OFFSET % alignof(OtelThreadCtxRecord) == 0, + "record must land on its natural alignment"); + +inline OtelThreadCtxRecord* CtxWrap::record() { + return reinterpret_cast( + reinterpret_cast(this) + RECORD_OFFSET); +} + +inline CtxWrap* CtxWrap::FromRecord(void* record) { + return reinterpret_cast(static_cast(record) - + RECORD_OFFSET); +} + +CtxWrap* CtxWrap::Create(size_t capacity) { + void* mem = calloc(1, RECORD_OFFSET + sizeof(OtelThreadCtxRecord) + capacity); + if (mem == nullptr) return nullptr; + return new (mem) CtxWrap(capacity); +} + +void CtxWrap::Destroy(CtxWrap* self) { + self->~CtxWrap(); + free(self); +} // Head of the live-CtxWrap list for this thread. Node pins each isolate to a // thread, and CtxWraps are only ever constructed and destroyed on their own @@ -293,13 +318,10 @@ static_assert(offsetof(CtxWrap, record_) == 0, // `otel_thread_ctx_nodejs_v1` above is thread-local for the same reason. thread_local CtxWrap* g_live_ctx_wraps = nullptr; -// Delete every CtxWrap V8 has not collected yet. This is the teardown deletion -// that node::ObjectWrap's per-instance cleanup hook used to provide; without it -// the records would simply leak at exit. Registered once per isolate from -// Init(), which runs at module initialisation with a context entered, so -// AddEnvironmentCleanupHook's own CHECK is satisfied, and never removed — it -// fires exactly once, at teardown, while the Environment is still alive. -void DrainLiveCtxWraps(void* arg) { +// Destroy every CtxWrap V8 has not collected yet. Without it the records would +// simply leak at exit. Registered once per isolate from Init(). It fires +// exactly once, at teardown, while the Environment is still alive. +void CtxWrap::DrainLive(void* arg) { auto* isolate = static_cast(arg); // We must allocate our own HandleScope here as node::FreeEnvironment wraps // RunCleanup in a SealHandleScope, so handle_.Get() below has to allocate @@ -312,8 +334,8 @@ void DrainLiveCtxWraps(void* arg) { CtxWrap* next = p->next_; p->pprev_ = nullptr; p->next_ = nullptr; - // Clear the holder's internal field (containing p as pointer value), so - // nothing can reach a dangling CtxWrap through it including the + // Clear the holder's internal field (containing p's record pointer), so + // nothing can reach a dangling record through it including the // out-of-process reader, which walks this slot. Being on the live list // means V8 has not collected the holder, so the handle is safe to read // here; the WeakCallback path cannot do this and does not need to, @@ -321,7 +343,7 @@ void DrainLiveCtxWraps(void* arg) { if (!p->handle_.IsEmpty()) { SetAlignedPointerInInternalField(p->handle_.Get(isolate), 0, nullptr); } - delete p; + Destroy(p); p = next; } g_live_ctx_wraps = nullptr; @@ -335,16 +357,15 @@ CtxWrap::~CtxWrap() { *pprev_ = next_; if (next_ != nullptr) next_->pprev_ = pprev_; } - free(record_); } void CtxWrap::WeakCallback(const v8::WeakCallbackInfo& data) { - delete data.GetParameter(); + Destroy(data.GetParameter()); } void CtxWrap::Wrap(Local holder) { Isolate* isolate = Isolate::GetCurrent(); - SetAlignedPointerInInternalField(holder, 0, this); + SetAlignedPointerInInternalField(holder, 0, record()); handle_.Reset(isolate, holder); handle_.SetWeak(this, &WeakCallback, v8::WeakCallbackType::kParameter); next_ = g_live_ctx_wraps; @@ -355,13 +376,14 @@ void CtxWrap::Wrap(Local holder) { CtxWrap* CtxWrap::Unwrap(Local holder) { if (holder->InternalFieldCount() < 1) return nullptr; - return static_cast(GetAlignedPointerFromInternalField(*holder, 0)); + void* record = GetAlignedPointerFromInternalField(*holder, 0); + if (record == nullptr) return nullptr; + return FromRecord(record); } -CtxWrap::CtxWrap(OtelThreadCtxRecord* record, size_t capacity, bool truncated) - : record_(record), - capacity_(capacity), - truncated_(truncated), +CtxWrap::CtxWrap(size_t capacity) + : capacity_(capacity), + truncated_(false), encoding_(false), pprev_(nullptr), next_(nullptr) {} @@ -512,12 +534,13 @@ void CtxWrap::New(const FunctionCallbackInfo& args) { // doesn't change the geometric-growth amortized cost of subsequent // appends). size_t capacity = std::max(attrs_buf.size(), MIN_INITIAL_CAPACITY); - const size_t total = sizeof(OtelThreadCtxRecord) + capacity; - OwnedRecord record(static_cast(calloc(1, total))); - if (!record) { + CtxWrap* self = CtxWrap::Create(capacity); + if (self == nullptr) { isolate->ThrowError("allocation failed"); return; } + self->truncated_ = truncated; + OtelThreadCtxRecord* record = self->record(); memcpy(record->trace_id, trace_id, sizeof(trace_id)); memcpy(record->span_id, span_id, sizeof(span_id)); record->attrs_data_size = static_cast(attrs_buf.size()); @@ -533,15 +556,14 @@ void CtxWrap::New(const FunctionCallbackInfo& args) { std::atomic_signal_fence(std::memory_order_release); *reinterpret_cast(&record->valid) = 1; - CtxWrap* self = new CtxWrap(record.release(), capacity, truncated); + // Only now does the record become reachable — Wrap() is what publishes + // its address into the holder's internal field. self->Wrap(args.This()); args.GetReturnValue().Set(args.This()); } -// Append entries to the active record. Either modifies the record in place -// (if the appended bytes fit in the current allocation's slack) or -// reallocates to a larger one (geometrically), keeping invariant -// `record_->attrs_data_size <= capacity_`. +// `appendAttributes(attributes)`: validate and encode the attributes, then +// hand the encoded bytes to AppendEncoded to splice onto the active record. void CtxWrap::Append(const FunctionCallbackInfo& args) { Isolate* isolate = args.GetIsolate(); Local context = isolate->GetCurrentContext(); @@ -556,27 +578,31 @@ void CtxWrap::Append(const FunctionCallbackInfo& args) { return; } - // Reject reentrant Append on the same wrap. EncodeAttrs' `ToString` - // below can execute user JS, and if that JS calls `appendAttributes` - // on this same ThreadContext, the reentrant call would grow - // attrs_data_size out from under the outer call's `current_used` - // snapshot, causing the outer memcpy to overwrite the reentrant call's - // bytes and the outer attrs_data_size write to shrink the record. + // Reject reentrant Append on the same wrap. EncodeAttrs' element getters + // and `ToString` below can execute user JS, and if that JS calls + // `appendAttributes` on this same ThreadContext, the reentrant call would + // grow attrs_data_size out from under the outer call's snapshot of it, + // causing the outer memcpy to overwrite the reentrant call's bytes and + // the outer attrs_data_size write to shrink the record. if (self->encoding_) { isolate->ThrowError( "reentrant appendAttributes on the same ThreadContext is not allowed"); return; } - self->encoding_ = true; - defer { - self->encoding_ = false; - }; - const size_t current_used = self->record_->attrs_data_size; std::vector appended; bool truncated = false; - if (!EncodeAttrs( - isolate, context, args[0], current_used, &appended, &truncated)) { + self->encoding_ = true; + const bool encoded = EncodeAttrs(isolate, + context, + args[0], + self->record()->attrs_data_size, + &appended, + &truncated); + // EncodeAttrs was the only thing that can run user JS, so the + // reentrancy guard had to span only it. + self->encoding_ = false; + if (!encoded) { return; } if (truncated) self->truncated_ = true; @@ -586,10 +612,23 @@ void CtxWrap::Append(const FunctionCallbackInfo& args) { // already at the cap. if (appended.empty()) return; + if (!self->AppendEncoded(args.This(), appended)) { + isolate->ThrowError("allocation failed"); + } +} + +// Splice `appended` onto the end of the record: in place if the bytes fit in +// the current allocation's slack, otherwise by moving the whole wrap to a +// larger allocation (grown geometrically), keeping invariant +// `record()->attrs_data_size <= capacity_`. See the declaration for the +// `this`-is-destroyed caveat on the growing path. +bool CtxWrap::AppendEncoded(Local holder, + const std::vector& appended) { + const size_t current_used = record()->attrs_data_size; const size_t new_used = current_used + appended.size(); // EncodeAttrs already enforced the cap; new_used <= MAX_ATTRS_DATA_SIZE. - if (new_used <= self->capacity_) { + if (new_used <= capacity_) { // In-place: write the new entries past the current attrs_data_size, // then bump attrs_data_size with a release fence + volatile store so // the content writes are visible before the size store from the @@ -601,55 +640,50 @@ void CtxWrap::Append(const FunctionCallbackInfo& args) { // mid-append sees either the old size (old extent, ignores the // half-written tail) or the new size (full new extent, all bytes // written). Either is consistent. - memcpy(&self->record_->attrs_data[current_used], - appended.data(), - appended.size()); + memcpy( + &record()->attrs_data[current_used], appended.data(), appended.size()); std::atomic_signal_fence(std::memory_order_release); - *reinterpret_cast(&self->record_->attrs_data_size) = + *reinterpret_cast(&record()->attrs_data_size) = static_cast(new_used); - return; + return true; } - // Doesn't fit. Reallocate with geometric growth with cap. + // Doesn't fit. Reallocate with geometric growth with cap. The record lives + // inside the CtxWrap allocation, so growing it means moving the CtxWrap + // too: build a replacement, hand it the same holder object, and retire the + // old one. size_t new_cap = - std::min(std::max(self->capacity_ * 2, new_used), MAX_ATTRS_DATA_SIZE); + std::min(std::max(capacity_ * 2, new_used), MAX_ATTRS_DATA_SIZE); - const size_t total = sizeof(OtelThreadCtxRecord) + new_cap; - OwnedRecord new_rec(static_cast(calloc(1, total))); - if (!new_rec) { - isolate->ThrowError("allocation failed"); - return; - } - // Capture before the copy: the point of the assert below is that the memcpy - // carried the header across intact, not that the record is valid. It used to - // assert `valid == 1`, which invalidate() legitimately makes false — and - // since NDEBUG is not defined for this addon, that aborted release builds - // too, not just debug ones. - const uint8_t src_valid = self->record_->valid; + CtxWrap* new_self = CtxWrap::Create(new_cap); + if (new_self == nullptr) return false; + new_self->truncated_ = truncated_; // Copy the existing record (header + already-written attrs_data). memcpy( - new_rec.get(), self->record_, sizeof(OtelThreadCtxRecord) + current_used); + new_self->record(), record(), sizeof(OtelThreadCtxRecord) + current_used); // Append the new entries and update attrs_data_size. - memcpy(&new_rec->attrs_data[current_used], appended.data(), appended.size()); - new_rec->attrs_data_size = static_cast(new_used); - // The copy should've carried the source record's header across verbatim, - // whatever its validity was. - assert(new_rec->valid == src_valid); - - // Publish: the pointer swap is the atomic boundary the reader sees. The - // first fence keeps the new_rec content writes ordered before the pointer - // store from the compiler's perspective. The second fence prevents free() - // from being hoisted above the pointer swap — without it, a reader stopped - // between a reordered free() and the not-yet-completed swap would follow - // self->record_ into freed memory. OTEP signal-handler semantics (the - // writer is stopped during reads) take care of CPU-side ordering and make - // immediate freeing of the old record safe. + memcpy(&new_self->record()->attrs_data[current_used], + appended.data(), + appended.size()); + new_self->record()->attrs_data_size = static_cast(new_used); + + // Publish: the internal-field store inside Wrap() is the atomic boundary + // the reader sees. The first fence keeps the new_self content writes + // ordered before that store from the compiler's perspective. The second + // fence prevents the free() inside Destroy() from being hoisted above it — + // without it, a reader stopped between a reordered free() and the + // not-yet-completed store would follow the internal field into freed + // memory. OTEP signal-handler semantics (the writer is stopped during + // reads) take care of CPU-side ordering and make immediate freeing of the + // old block safe. std::atomic_signal_fence(std::memory_order_release); - OtelThreadCtxRecord* old_rec = self->record_; - self->record_ = new_rec.release(); - self->capacity_ = new_cap; + new_self->Wrap(holder); std::atomic_signal_fence(std::memory_order_acq_rel); - free(old_rec); + // Destroying the old wrap runs ~Global on its handle_, which resets the + // weak handle and so cancels the WeakCallback V8 would otherwise fire on + // the freed block. + CtxWrap::Destroy(this); + return true; } // Mark this record's `valid` byte as 0 in place. Every async-context @@ -668,7 +702,7 @@ void CtxWrap::Invalidate(const FunctionCallbackInfo& args) { return; } std::atomic_signal_fence(std::memory_order_release); - *reinterpret_cast(&self->record_->valid) = 0; + *reinterpret_cast(&self->record()->valid) = 0; } // Returns true if any attribute was ever dropped from this wrapper's @@ -695,15 +729,15 @@ void CtxWrap::DebugBytes(const FunctionCallbackInfo& args) { return; } const size_t total = - sizeof(OtelThreadCtxRecord) + self->record_->attrs_data_size; + sizeof(OtelThreadCtxRecord) + self->record()->attrs_data_size; Local buf = v8::ArrayBuffer::New(isolate, total); - memcpy(buf->GetBackingStore()->Data(), self->record_, total); + memcpy(buf->GetBackingStore()->Data(), self->record(), total); args.GetReturnValue().Set(Uint8Array::New(buf, 0, total)); } void CtxWrap::Init(Local exports) { Isolate* isolate = Isolate::GetCurrent(); - node::AddEnvironmentCleanupHook(isolate, DrainLiveCtxWraps, isolate); + node::AddEnvironmentCleanupHook(isolate, DrainLive, isolate); Local context = isolate->GetCurrentContext(); Local tpl = FunctionTemplate::New(isolate, New); @@ -794,9 +828,11 @@ void GetStoredAlsHash(const FunctionCallbackInfo& args) { // V8 layout constants captured at addon-compile time from the same V8 // headers Node bundles. Published via the discovery contract so an -// out-of-process reader can decode our wrapper / V8's internal hashmap +// out-of-process reader can decode V8's JSObject / internal hashmap // layout without doing its own V8-internal-symbol lookups for the -// pointer-compression / sandbox state. +// pointer-compression / sandbox state. Note that nothing published here +// describes our own wrapper: internal field 0 points straight at the +// record, so the reader needs no offset of ours to reach it. #if NODE_MAJOR_VERSION >= 22 constexpr int WRAPPED_OBJECT_OFFSET = v8::internal::Internals::kJSObjectHeaderSize + @@ -811,14 +847,6 @@ constexpr int WRAPPED_OBJECT_OFFSET = 0; #endif constexpr int TAGGED_SIZE = v8::internal::kApiTaggedSize; -// Given a pointer to a CtxWrap — reached from the JSObject's V8 -// wrapped-object slot — add this offset to arrive at `record_`. CtxWrap has -// no base class, so `record_` is its first member and the offset is zero; -// computing it with offsetof keeps the published value correct if the layout -// ever changes again. -constexpr int NATIVE_WRAP_FIELDS_OFFSET = - static_cast(offsetof(CtxWrap, record_)); - // V8 JSMap layout: kTableOffset within the JSMap object holds a tagged // pointer to the backing OrderedHashMap table. Not exposed in V8's // public headers; kept in sync with @@ -850,7 +878,6 @@ void OtelThreadCtx::Init(Local exports) { .FromJust(); }; publish_int("otelThreadCtxJsMapTableOffset", JS_MAP_TABLE_OFFSET); - publish_int("otelThreadCtxNativeWrapFieldsOffset", NATIVE_WRAP_FIELDS_OFFSET); publish_int("otelThreadCtxOrderedHashMapHeaderSize", ORDERED_HASH_MAP_HEADER_SIZE); publish_int("otelThreadCtxTaggedSize", TAGGED_SIZE); diff --git a/ts/src/otel-thread-ctx.ts b/ts/src/otel-thread-ctx.ts index 6ee061ed..101ca9e5 100644 --- a/ts/src/otel-thread-ctx.ts +++ b/ts/src/otel-thread-ctx.ts @@ -50,7 +50,6 @@ export interface ProcessContextAttributes { readonly 'threadlocal.attribute_key_map': readonly string[]; readonly 'threadlocal.wrapped_object_offset': number; readonly 'threadlocal.tagged_size': number; - readonly 'threadlocal.native_wrap_fields_offset': number; readonly 'threadlocal.js_map_table_offset': number; readonly 'threadlocal.ordered_hash_map_header_size': number; } @@ -63,9 +62,9 @@ export interface ProcessContextAttributes { * * `appendAttributes` mutates the context's record in place. Because every * async-context frame that holds the same `ThreadContext` reference observes - * the same native record buffer, an append is visible across all those - * frames even when the reallocate path runs (the context's internal - * pointer is updated, the JS object is not replaced). + * the same native record, an append is visible across all those frames even + * when the reallocate path runs (the record is re-published on the same JS + * object, which is never replaced). */ export interface ThreadContext { appendAttributes( @@ -130,7 +129,6 @@ interface Addon { otelThreadCtxGetStoredAlsHash(): number; otelThreadCtxWrappedObjectOffset: number; otelThreadCtxTaggedSize: number; - otelThreadCtxNativeWrapFieldsOffset: number; otelThreadCtxJsMapTableOffset: number; otelThreadCtxOrderedHashMapHeaderSize: number; } @@ -144,7 +142,6 @@ const SCHEMA_VERSION = 'nodejs_v1_dev'; // consistent in shape. let WRAPPED_OBJECT_OFFSET = 24; let TAGGED_SIZE = 8; -let NATIVE_WRAP_FIELDS_OFFSET = 0; let JS_MAP_TABLE_OFFSET = 0x18; let ORDERED_HASH_MAP_HEADER_SIZE = 0x10; @@ -173,7 +170,6 @@ if (process.platform === 'linux') { const addon: Addon = findBinding(join(__dirname, '..', '..')); WRAPPED_OBJECT_OFFSET = addon.otelThreadCtxWrappedObjectOffset; TAGGED_SIZE = addon.otelThreadCtxTaggedSize; - NATIVE_WRAP_FIELDS_OFFSET = addon.otelThreadCtxNativeWrapFieldsOffset; JS_MAP_TABLE_OFFSET = addon.otelThreadCtxJsMapTableOffset; ORDERED_HASH_MAP_HEADER_SIZE = addon.otelThreadCtxOrderedHashMapHeaderSize; @@ -288,7 +284,6 @@ export function getProcessContextAttributes( 'threadlocal.attribute_key_map': Object.freeze(keys.slice()), 'threadlocal.wrapped_object_offset': WRAPPED_OBJECT_OFFSET, 'threadlocal.tagged_size': TAGGED_SIZE, - 'threadlocal.native_wrap_fields_offset': NATIVE_WRAP_FIELDS_OFFSET, 'threadlocal.js_map_table_offset': JS_MAP_TABLE_OFFSET, 'threadlocal.ordered_hash_map_header_size': ORDERED_HASH_MAP_HEADER_SIZE, }) as ProcessContextAttributes; diff --git a/ts/test/test-otel-thread-ctx.ts b/ts/test/test-otel-thread-ctx.ts index e28a6f03..c2e95ab1 100644 --- a/ts/test/test-otel-thread-ctx.ts +++ b/ts/test/test-otel-thread-ctx.ts @@ -839,7 +839,6 @@ function captureBytes(opts: { strictAssert.deepEqual(pca['threadlocal.attribute_key_map'], keys); strictAssert.equal(pca['threadlocal.wrapped_object_offset'], 24); strictAssert.equal(pca['threadlocal.tagged_size'], 8); - strictAssert.equal(pca['threadlocal.native_wrap_fields_offset'], 0); strictAssert.equal(pca['threadlocal.js_map_table_offset'], 0x18); strictAssert.equal( pca['threadlocal.ordered_hash_map_header_size'], @@ -848,7 +847,6 @@ function captureBytes(opts: { strictAssert.deepEqual(Object.keys(pca).sort(), [ 'threadlocal.attribute_key_map', 'threadlocal.js_map_table_offset', - 'threadlocal.native_wrap_fields_offset', 'threadlocal.ordered_hash_map_header_size', 'threadlocal.schema_version', 'threadlocal.tagged_size',