Skip to content

Commit 355f10d

Browse files
joyeecheungruyadorno
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. Backport-PR-URL: #49874 PR-URL: #49671 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Michaël Zasso <targos@protonmail.com>
1 parent 17cfc53 commit 355f10d

4 files changed

Lines changed: 20 additions & 33 deletions

File tree

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

Lines changed: 5 additions & 8 deletions
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
constvm=require('vm');
8-
letcount=0;
99

10-
functionmain(){
11-
// Try to reach the maximum old space size.
12-
vm.compileFunction(`"${Math.random().toString().repeat(512)}"`,[],{
10+
asyncfunctioncreateCompiledFunction(){
11+
returnvm.compileFunction(`"${Math.random().toString().repeat(512)}"`,[],{
1312
asyncimportModuleDynamically(){},
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‎

Lines changed: 4 additions & 7 deletions
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
constvm=require('vm');
8-
letcount=0;
99

10-
functionmain(){
10+
asyncfunctioncreateContextifyScript(){
1111
// Try to reach the maximum old space size.
12-
newvm.Script(`"${Math.random().toString().repeat(512)}";`,{
12+
returnnewvm.Script(`"${Math.random().toString().repeat(512)}";`,{
1313
asyncimportModuleDynamically(){},
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‎

Lines changed: 8 additions & 10 deletions
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
constvm=require('vm');
9-
letcount=0;
10-
asyncfunctioncreateModule(){
9+
10+
asyncfunctioncreateSourceTextModule(){
1111
// Try to reach the maximum old space size.
1212
constm=newvm.SourceTextModule(`
13-
const bar = new Array(512).fill("----");
14-
export { bar };
15-
`);
13+
const bar = new Array(512).fill("----");
14+
export { bar };
15+
`);
1616
awaitm.link(()=>{});
1717
awaitm.evaluate();
18-
if(count++<4096){
19-
setTimeout(createModule,1);
20-
}
2118
returnm;
2219
}
23-
createModule();
20+
21+
checkIfCollectable(createSourceTextModule,4096,1024);

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

Lines changed: 3 additions & 8 deletions
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
constvm=require('vm');
89

9-
letcount=0;
10-
asyncfunctioncreateModule(){
11-
// Try to reach the maximum old space size.
10+
asyncfunctioncreateSyntheticModule(){
1211
constm=newvm.SyntheticModule(['bar'],()=>{
1312
m.setExport('bar',newArray(512).fill('----'));
1413
});
1514
awaitm.link(()=>{});
1615
awaitm.evaluate();
17-
if(count++<4*1024){
18-
setTimeout(createModule,1);
19-
}
2016
returnm;
2117
}
22-
23-
createModule();
18+
checkIfCollectable(createSyntheticModule,4096);

0 commit comments

Comments
 (0)