fix(otel-thread-ctx): don't derive CtxWrap from node::ObjectWrap

[szegedi]

CtxWrap has the same defect #385 fixed in the wall profiler's PersistentContextPtr.

The abort

node::ObjectWrap registers a per-instance environment cleanup hook in its constructor and calls RemoveEnvironmentCleanupHook from its destructor, which CHECKs that an Environment is current. A CtxWrap is owned by a weak V8 handle, so V8 picks when it dies — and weak callbacks run during isolate teardown with no context entered:

Assertion failed: (env) != nullptr
 2: node::RemoveEnvironmentCleanupHook(...)
 3: otel_thread_ctx_nodejs::CtxWrap::~CtxWrap()

This one is not subtle. Create a few thousand ThreadContexts and exit normally:

N	before
1, 10, 100	exit 0
1000, 3000	exit 134

Every run, on a plain release build — no ASAN required, unlike the PCP case. Nothing below ~1000 reproduces it: V8 has to still have instances left to collect at teardown. ThreadContext is public API, so any Linux consumer creating contexts at volume aborts at process exit.

Why not just skip the removal

Worth being explicit, because it's the tempting fix and it's wrong. The CHECK guards something real:

inline Environment* Environment::GetCurrent(v8::Isolate* isolate) {
  if (!isolate->InContext()) [[unlikely]] return nullptr;

Null means "no context entered", not "no environment" — the Environment may well be alive. Leaving a hook behind whose arg is a freed pointer turns the abort into a use-after-free when CleanupQueue::Drain later calls cb.fn_(cb.arg_). So the fix is to never register the per-instance hook, not to bypass its removal.

Replacing what the hook did

Dropping the base loses the one thing it provided: deletion at teardown even when V8 never collects the object. PersistentContextPtr could lean on ~WallProfiler walking its live list — CtxWrap has no such owner and owns a malloc'd record_, so a naive port would trade the abort for a leak.

So this adds the equivalent: a thread-local list of live CtxWraps drained by one per-isolate cleanup hook instead of one per instance. It's registered from Wrap() — inside a JS constructor call, where a context is entered, so AddEnvironmentCleanupHook's own CHECK is satisfied honestly — and never removed, since it fires exactly once at teardown while the Environment is alive. Removal timing we control, rather than V8.

Thread-local rather than locked, for the same reason the wall profiler's active-profiler pointer is: Node pins each isolate to a thread, and CtxWraps are only constructed and destroyed on their own isolate's thread. otel_thread_ctx_nodejs_v1 in the same file is thread-local on the same grounds.

offsetof / -Winvalid-offsetof

Losing the base also makes CtxWrap standard-layout — no base subobject, no virtuals, all data members in one access section — so offsetof on it is now unconditionally valid, and the two -Winvalid-offsetof suppressions the inheriting version needed are gone. A static_assert(std::is_standard_layout<CtxWrap>::value) keeps it that way, since the reader contract depends on offsetof(record_) being well-defined. Verified with both clang and gcc 12: builds clean, zero offsetof warnings.

Reader contract

With no base class, record_ is CtxWrap's first member, so threadlocal.native_wrap_fields_offset goes 24 → 0, and is now computed with offsetof rather than sizeof() of a foreign type we don't control. That's a breaking reader-contract change, done now because no readers exist yet. SCHEMA_VERSION is left at nodejs_v1_dev; say the word if you'd rather mark it.

internal-field.hh

Node 26 requires an EmbedderDataTypeTag on both the get and the set of an internal field — the thing that broke my first attempt at #385. The pair now lives in one header so they can't drift when only one is exercised on the version you happen to build against. wall.cc keeps its own copies for now to avoid conflicting with #387; folding those in is a follow-up.

Verification

	result
new test vs pre-fix binding	exited with code=null signal=SIGABRT
new test vs fixed binding	passing
Node 26 npm test	exit 0, 164 passing, 0 aborts
Node 24 test:js-asan	exit 0, 164 passing, 0 leaks, 0 aborts
Node 20 test:js-asan	exit 0, 100 passing, 0 leaks, 0 aborts
probe at N=1000 / 3000 / 10000	all exit 0

The zero leak counts are the load-bearing check: they confirm the drain hook really does replace what ObjectWrap was doing, rather than just moving the failure.

The regression test forks, since the failure is a SIGABRT that would otherwise take the whole mocha run down.

Follow-up

The same defect is present in the upstream custom-labels copy (js/addon.cpp) on both otel-thread-ctx-wip and otel-thread-ctx-invalidate — I reproduced it there with the same probe and thresholds. This PR is intended as the reference implementation to port.

[github-actions]

Overall package size

Self size: 2.46 MB
Deduped: 3.17 MB
No deduping: 3.17 MB

Dependency sizes

| name | version | self size | total size | |------|---------|-----------|------------| | pprof-format | 2.3.0 | 503.97 kB | 503.97 kB | | source-map | 0.8.0 | 185.66 kB | 185.66 kB | | node-gyp-build | 4.8.4 | 13.86 kB | 13.86 kB |

_{🤖 This report was automatically generated by heaviest-objects-in-the-universe}

[datadog-official]

 

⚠️ Warnings

🚦 3 Pipeline jobs failed

Build | build / darwin-arm64-test-22     

Build | build / win32-test-22     

Build | build-successful     

ℹ️ Info

🔄 Datadog auto-retried 1 job - 1 passed on retry

Useful? React with 👍 / 👎

_{This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 54e10ab | Docs | Datadog PR Page | Give us feedback!}