Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: add and use checkIfCollectable in vm leak tests #49671

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
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.
  • Loading branch information
joyeecheung committed Sep 16, 2023
commit 6174c98911470f1422c8009b824971ebf5885234
13 changes: 5 additions & 8 deletions test/es-module/test-vm-compile-function-leak.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,13 @@
// This tests that vm.compileFunction with dynamic import callback does not leak.
// See https://github.com/nodejs/node/issues/44211
require('../common');
const { checkIfCollectable } = require('../common/gc');
const vm = require('vm');
let count = 0;

function main() {
// Try to reach the maximum old space size.
vm.compileFunction(`"${Math.random().toString().repeat(512)}"`, [], {
async function createCompiledFunction() {
return vm.compileFunction(`"${Math.random().toString().repeat(512)}"`, [], {
async importModuleDynamically() {},
});
if (count++ < 2048) {
setTimeout(main, 1);
}
}
main();

checkIfCollectable(createCompiledFunction, 2048);
11 changes: 4 additions & 7 deletions test/es-module/test-vm-contextified-script-leak.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,13 @@
// This tests that vm.Script with dynamic import callback does not leak.
// See: https://github.com/nodejs/node/issues/33439
require('../common');
const { checkIfCollectable } = require('../common/gc');
const vm = require('vm');
let count = 0;

function main() {
async function createContextifyScript() {
// Try to reach the maximum old space size.
new vm.Script(`"${Math.random().toString().repeat(512)}";`, {
return new vm.Script(`"${Math.random().toString().repeat(512)}";`, {
async importModuleDynamically() {},
});
if (count++ < 2 * 1024) {
setTimeout(main, 1);
}
}
main();
checkIfCollectable(createContextifyScript, 2048);
18 changes: 8 additions & 10 deletions test/es-module/test-vm-source-text-module-leak.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,18 @@
// This tests that vm.SourceTextModule() does not leak.
// See: https://github.com/nodejs/node/issues/33439
require('../common');

const { checkIfCollectable } = require('../common/gc');
const vm = require('vm');
let count = 0;
async function createModule() {

async function createSourceTextModule() {
// Try to reach the maximum old space size.
const m = new vm.SourceTextModule(`
const bar = new Array(512).fill("----");
export { bar };
`);
const bar = new Array(512).fill("----");
export { bar };
`);
await m.link(() => {});
await m.evaluate();
if (count++ < 4096) {
setTimeout(createModule, 1);
}
return m;
}
createModule();

checkIfCollectable(createSourceTextModule, 4096);
11 changes: 3 additions & 8 deletions test/es-module/test-vm-synthetic-module-leak.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,15 @@
// This tests that vm.SyntheticModule does not leak.
// See https://github.com/nodejs/node/issues/44211
require('../common');
const { checkIfCollectable } = require('../common/gc');
const vm = require('vm');

let count = 0;
async function createModule() {
// Try to reach the maximum old space size.
async function createSyntheticModule() {
const m = new vm.SyntheticModule(['bar'], () => {
m.setExport('bar', new Array(512).fill('----'));
});
await m.link(() => {});
await m.evaluate();
if (count++ < 4 * 1024) {
setTimeout(createModule, 1);
}
return m;
}

createModule();
checkIfCollectable(createSyntheticModule, 4096);
Loading