Skip to content

Commit 0b156c6

Browse files
joyeecheungrichardlau
authored andcommitted
test: use checkIfCollectable in vm leak tests
Previously we simply create a lot of the target objects and check if the process crash due to OOM. Due to how we use emphemeron GC to handle memory management, which is inefficient but necessary for correctness, the tests can produce false positives as the GC isn't efficient enough to catch up with a very fast heap growth. This patch uses a new checkIfCollectable() utility to terminate the test early once we detect that any of the target object can actually be garbage collected. This should lower the chance of false positives. As a drive-by this also allows us to use setImmediate() to grow the heap even faster to make the tests run faster. PR-URL: #49671 Backport-PR-URL: #51004 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>
1 parent 1586c11 commit 0b156c6

4 files changed

+20
-33
lines changed

test/es-module/test-vm-compile-function-leak.js

+5-8
Original file line numberDiff line numberDiff line change
@@ -4,16 +4,13 @@
44
// This tests that vm.compileFunction with dynamic import callback does not leak.
55
// See https://github.com/nodejs/node/issues/44211
66
require('../common');
7+
const { checkIfCollectable } = require('../common/gc');
78
const vm = require('vm');
8-
let count = 0;
99

10-
function main() {
11-
// Try to reach the maximum old space size.
12-
vm.compileFunction(`"${Math.random().toString().repeat(512)}"`, [], {
10+
async function createCompiledFunction() {
11+
return vm.compileFunction(`"${Math.random().toString().repeat(512)}"`, [], {
1312
async importModuleDynamically() {},
1413
});
15-
if (count++ < 2048) {
16-
setTimeout(main, 1);
17-
}
1814
}
19-
main();
15+
16+
checkIfCollectable(createCompiledFunction, 2048);

test/es-module/test-vm-contextified-script-leak.js

+4-7
Original file line numberDiff line numberDiff line change
@@ -4,16 +4,13 @@
44
// This tests that vm.Script with dynamic import callback does not leak.
55
// See: https://github.com/nodejs/node/issues/33439
66
require('../common');
7+
const { checkIfCollectable } = require('../common/gc');
78
const vm = require('vm');
8-
let count = 0;
99

10-
function main() {
10+
async function createContextifyScript() {
1111
// Try to reach the maximum old space size.
12-
new vm.Script(`"${Math.random().toString().repeat(512)}";`, {
12+
return new vm.Script(`"${Math.random().toString().repeat(512)}";`, {
1313
async importModuleDynamically() {},
1414
});
15-
if (count++ < 2 * 1024) {
16-
setTimeout(main, 1);
17-
}
1815
}
19-
main();
16+
checkIfCollectable(createContextifyScript, 2048);

test/es-module/test-vm-source-text-module-leak.js

+8-10
Original file line numberDiff line numberDiff line change
@@ -4,20 +4,18 @@
44
// This tests that vm.SourceTextModule() does not leak.
55
// See: https://github.com/nodejs/node/issues/33439
66
require('../common');
7-
7+
const { checkIfCollectable } = require('../common/gc');
88
const vm = require('vm');
9-
let count = 0;
10-
async function createModule() {
9+
10+
async function createSourceTextModule() {
1111
// Try to reach the maximum old space size.
1212
const m = new vm.SourceTextModule(`
13-
const bar = new Array(512).fill("----");
14-
export { bar };
15-
`);
13+
const bar = new Array(512).fill("----");
14+
export { bar };
15+
`);
1616
await m.link(() => {});
1717
await m.evaluate();
18-
if (count++ < 4096) {
19-
setTimeout(createModule, 1);
20-
}
2118
return m;
2219
}
23-
createModule();
20+
21+
checkIfCollectable(createSourceTextModule, 4096, 1024);

test/es-module/test-vm-synthetic-module-leak.js

+3-8
Original file line numberDiff line numberDiff line change
@@ -4,20 +4,15 @@
44
// This tests that vm.SyntheticModule does not leak.
55
// See https://github.com/nodejs/node/issues/44211
66
require('../common');
7+
const { checkIfCollectable } = require('../common/gc');
78
const vm = require('vm');
89

9-
let count = 0;
10-
async function createModule() {
11-
// Try to reach the maximum old space size.
10+
async function createSyntheticModule() {
1211
const m = new vm.SyntheticModule(['bar'], () => {
1312
m.setExport('bar', new Array(512).fill('----'));
1413
});
1514
await m.link(() => {});
1615
await m.evaluate();
17-
if (count++ < 4 * 1024) {
18-
setTimeout(createModule, 1);
19-
}
2016
return m;
2117
}
22-
23-
createModule();
18+
checkIfCollectable(createSyntheticModule, 4096);

0 commit comments

Comments
 (0)