Skip to content

Commit 4677f54

Browse files
committed
Add a regression test for the CtxWrap teardown abort
Covers the abort fixed in the previous commit: CtxWrap derived from node::ObjectWrap, whose destructor calls RemoveEnvironmentCleanupHook, which CHECKs that an Environment is current. A CtxWrap collected during isolate teardown hit that CHECK and aborted the process. It runs in a spawned process because the failure is a SIGABRT: `node --test` runs the file in-process, so an abort takes the whole file down rather than failing one test. Against the pre-fix addon the run reports `pass 0, fail 1` with the CtxWrap::~CtxWrap stack rather than one failing test among the passes, which is why testing this inline was not an option. Spawning needs the same version-dependent flag list as the runner, so it uses acfFlags() from test/node-flags.js.
1 parent f21ae4b commit 4677f54

2 files changed

Lines changed: 68 additions & 0 deletions

File tree

js/test/teardown-child.js

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
'use strict';
2+
3+
// Spawned by the "contexts collected during isolate teardown" test. Runs in
4+
// its own process because the failure mode is a SIGABRT, which would take the
5+
// whole test run down with it.
6+
//
7+
// When CtxWrap derived from node::ObjectWrap, a CtxWrap collected during
8+
// isolate teardown ran ~ObjectWrap -> RemoveEnvironmentCleanupHook, which
9+
// CHECKs that an Environment is current. It is not, during teardown, so:
10+
//
11+
// Assertion failed: (env) != nullptr
12+
// 3: otel_thread_ctx_nodejs::CtxWrap::~CtxWrap()
13+
//
14+
// It needs enough instances that V8 still has some left to collect at
15+
// teardown — nothing below ~1000 reproduced it — hence the count.
16+
17+
const { ThreadContext } = require('..');
18+
19+
const N = Number(process.argv[2] || 3000);
20+
21+
function id(n, len) {
22+
const b = Buffer.alloc(len);
23+
b.writeUInt32BE(n >>> 0, 0);
24+
return b;
25+
}
26+
27+
const retained = [];
28+
29+
for (let i = 0; i < N; i++) {
30+
const ctx = new ThreadContext(id(i, 16), id(i, 8), ['k', String(i)]);
31+
if (i % 4 === 0) {
32+
// Still strongly reachable at exit.
33+
retained.push(ctx);
34+
} else {
35+
// Reachable only through the async context frame, so collectable
36+
// whenever V8 decides — including during teardown.
37+
ctx.enter();
38+
}
39+
}
40+
41+
if (retained.length > 0) {
42+
retained[0].enter();
43+
}
44+
globalThis.__retained = retained;
45+
46+
// Exit through the normal path so the Environment is torn down and the
47+
// isolate disposed; that is where the weak callbacks in question fire.
48+
console.log(`created ${N}, retained ${retained.length}`);

js/test/test.js

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@ if (!isAsyncContextFrameAvailable()) {
3333
const path = require('node:path');
3434
const { spawnSync } = require('node:child_process');
3535

36+
const { acfFlags } = require('./node-flags');
37+
3638
const lib = require('..');
3739
const { ThreadContext, getContext, clearContext, getProcessContextAttributes, _currentRecordBytes } = lib;
3840

@@ -597,6 +599,24 @@ test('appendAttributes after invalidate mutates attrs_data but leaves valid=0',
597599
});
598600
});
599601

602+
// Regression test: CtxWrap used to derive from node::ObjectWrap, whose
603+
// destructor calls RemoveEnvironmentCleanupHook. A CtxWrap collected during
604+
// isolate teardown hit that function's CHECK that an Environment is current
605+
// and aborted the process. Spawned, because the failure is a SIGABRT rather
606+
// than an assertion failure.
607+
test('contexts collected during isolate teardown do not abort', () => {
608+
const child = path.join(__dirname, 'teardown-child.js');
609+
const r = spawnSync(process.execPath, [...acfFlags(), child, '3000'], {
610+
encoding: 'utf8',
611+
});
612+
assert.equal(
613+
r.status,
614+
0,
615+
`teardown-child exited with status=${r.status} signal=${r.signal}\n` +
616+
`${r.stdout}${r.stderr}`,
617+
);
618+
});
619+
600620
test('otel_thread_ctx_nodejs_v1 is exported as a TLS dynsym', (t) => {
601621
const addon = path.join(__dirname, '..', 'build', 'Release', 'customlabels.node');
602622
if (!require('node:fs').existsSync(addon)) {

0 commit comments

Comments
 (0)