Skip to content

Commit 604a480

Browse files
committed
Add a regression test for the CtxWrap teardown abort
1 parent e119782 commit 604a480

2 files changed

Lines changed: 67 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 (~1000) that V8 still has some left to collect
15+
// at teardown.
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: 19 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

@@ -595,6 +597,23 @@ test('appendAttributes after invalidate mutates attrs_data but leaves valid=0',
595597
});
596598
});
597599

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

0 commit comments

Comments
 (0)