From 466c30498ed05474d4d32793332fc6cdf55cdcad Mon Sep 17 00:00:00 2001
From: Joyee Cheung <joyeec9h3@gmail.com>
Date: Sat, 16 Sep 2023 15:08:18 +0200
Subject: [PATCH] test: use checkIfCollectable in vm leak tests
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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: https://github.com/nodejs/node/pull/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>
---
 .../es-module/test-vm-compile-function-leak.js | 13 +++++--------
 .../test-vm-contextified-script-leak.js        | 11 ++++-------
 .../test-vm-source-text-module-leak.js         | 18 ++++++++----------
 .../es-module/test-vm-synthetic-module-leak.js | 11 +++--------
 4 files changed, 20 insertions(+), 33 deletions(-)

diff --git a/test/es-module/test-vm-compile-function-leak.js b/test/es-module/test-vm-compile-function-leak.js
index ff061cdaec7a01..f9f04588fdc7c3 100644
--- a/test/es-module/test-vm-compile-function-leak.js
+++ b/test/es-module/test-vm-compile-function-leak.js
@@ -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);
diff --git a/test/es-module/test-vm-contextified-script-leak.js b/test/es-module/test-vm-contextified-script-leak.js
index 7498b46ab80cfa..60212dd4bbbf68 100644
--- a/test/es-module/test-vm-contextified-script-leak.js
+++ b/test/es-module/test-vm-contextified-script-leak.js
@@ -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);
diff --git a/test/es-module/test-vm-source-text-module-leak.js b/test/es-module/test-vm-source-text-module-leak.js
index bf7f70c670e34c..d05e812ac32c95 100644
--- a/test/es-module/test-vm-source-text-module-leak.js
+++ b/test/es-module/test-vm-source-text-module-leak.js
@@ -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, 1024);
diff --git a/test/es-module/test-vm-synthetic-module-leak.js b/test/es-module/test-vm-synthetic-module-leak.js
index 9de02cb22f1128..bc0e4689535327 100644
--- a/test/es-module/test-vm-synthetic-module-leak.js
+++ b/test/es-module/test-vm-synthetic-module-leak.js
@@ -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);