fixup! module: add clearCache for CJS and ESM

Author: anonrig

doc/api/module.md

@@ -111,6 +111,36 @@ same target are not cleared — only entries for the exact `(specifier, parentUR
 are removed. The module cache itself is cleared by resolved file path, so all specifiers
 pointing to the same file will see a fresh execution on next import.
 
+#### Memory retention and static imports
+
+`clearCache` only removes references from the Node.js **JavaScript-level** caches
+(the ESM load cache, resolve cache, CJS `require.cache`, and related structures).
+It does **not** affect V8-internal module graph references.
+
+When a module M is **statically imported** by a live parent module P
+(i.e., via a top-level `import … from '…'` statement that has already been
+evaluated), V8's module instantiation creates a permanent internal strong
+reference from P's compiled module record to M's module record. Calling
+`clearCache(M)` cannot sever that link. Consequences:
+
+* The old instance of M **stays alive in memory** for as long as P is alive,
+  regardless of how many times M is cleared and re-imported.
+* A fresh `import(M)` after clearing will create a **separate** module instance
+  that new importers see. P, however, continues to use the original instance —
+  the two coexist simultaneously (sometimes called a "split-brain" state).
+* This is a **bounded** retention: one stale module instance per cleared module
+  per live static parent. It does not grow unboundedly across clear/re-import
+  cycles.
+
+For **dynamically imported** modules (`await import('./M.mjs')` with no live
+static parent holding the result), the old `ModuleWrap` becomes eligible for
+garbage collection once `clearCache` removes it from Node.js caches and all
+JS-land references (e.g., stored namespace objects) are dropped.
+
+The safest pattern for hot-reload of ES modules is to use cache-busting search
+parameters (so each version is a distinct module URL) and to avoid statically
+importing modules that need to be reloaded:
+
 #### ECMA-262 spec considerations
 
 Re-importing the exact same `(specifier, parentURL, importAttributes)` tuple after clearing the module cache

test/es-module/test-module-clear-cache-hash-map.mjs

@@ -0,0 +1,115 @@
+// Flags: --expose-internals
+// Evaluates the hash_to_module_map memory behaviour across clearCache cycles.
+//
+// hash_to_module_map is a C++ unordered_multimap<int, ModuleWrap*> on the
+// Environment. Every new ModuleWrap adds an entry; the destructor removes it.
+// Clearing the Node-side loadCache does not directly touch hash_to_module_map —
+// entries are removed only when ModuleWrap objects are garbage-collected.
+//
+// We verify two invariants:
+//
+//  1. DYNAMIC imports: after clearCache + GC, the old ModuleWrap is collected
+//     and therefore its hash_to_module_map entry is removed. The map does NOT
+//     grow without bound for purely-dynamic import/clear cycles.
+//     (Verified via checkIfCollectableByCounting.)
+//
+//  2. STATIC imports: when a parent P statically imports M, clearing M from
+//     the load cache does not free M's ModuleWrap (the static link keeps it).
+//     Each re-import adds one new entry while the old entry stays for the
+//     lifetime of P. This is a bounded, expected retention (not an unbounded
+//     leak): it is capped at one stale entry per module per live static parent.
+
+import '../common/index.mjs';
+
+import assert from 'node:assert';
+import { clearCache, createRequire } from 'node:module';
+import { queryObjects } from 'v8';
+
+const require = createRequire(import.meta.url);
+const { checkIfCollectableByCounting } = require('../common/gc');
+const { internalBinding } = require('internal/test/binding');
+const { ModuleWrap } = internalBinding('module_wrap');
+
+const counterBase = new URL(
+  '../fixtures/module-cache/esm-counter.mjs',
+  import.meta.url,
+).href;
+
+const parentURL = new URL(
+  '../fixtures/module-cache/esm-static-parent.mjs',
+  import.meta.url,
+).href;
+
+// ── Invariant 1: dynamic-only cycles do NOT leak ModuleWraps ────────────────
+// Use cache-busting query params so each import gets a distinct URL.
+
+const outer = 8;
+const inner = 4;
+
+await checkIfCollectableByCounting(async (i) => {
+  for (let j = 0; j < inner; j++) {
+    const url = `${counterBase}?hm=${i}-${j}`;
+    await import(url);
+    clearCache(url, {
+      parentURL: import.meta.url,
+      resolver: 'import',
+      caches: 'all',
+    });
+  }
+  return inner;
+}, ModuleWrap, outer, 50);
+
+// ── Invariant 2: static-parent cycles cause bounded retention ───────────────
+// After loading the static parent (which pins one counter instance), each
+// clear+re-import of the base counter URL creates exactly one new ModuleWrap
+// while the old one stays alive (pinned by the parent).
+// The net growth per cycle is +1. After N cycles the live count is
+//   baseline + 1(parent) + 1(pinned original counter) + 1(current counter)
+// — a constant overhead, not growing with N.
+
+// Load the static parent; this also loads the counter (count starts at 1 for
+// the global, but we seed it fresh by clearing any earlier runs' state).
+delete globalThis.__module_cache_esm_counter;
+
+const parent = await import(parentURL);
+assert.strictEqual(parent.count, 1);
+
+const wrapCount0 = queryObjects(ModuleWrap, { format: 'count' });
+
+// Cycle 1: clear counter + re-import → new instance created, old pinned.
+clearCache(counterBase, {
+  parentURL: import.meta.url,
+  resolver: 'import',
+  caches: 'all',
+});
+const v2 = await import(counterBase);
+assert.strictEqual(v2.count, 2);
+
+const wrapCount1 = queryObjects(ModuleWrap, { format: 'count' });
+// +1 new ModuleWrap (v2); old one kept alive by parent's static link.
+assert.strictEqual(wrapCount1, wrapCount0 + 1,
+                   'Each clear+reimport cycle adds exactly one new ModuleWrap ' +
+                   'when a static parent holds the old instance');
+
+// Cycle 2: clear counter again + re-import.
+clearCache(counterBase, {
+  parentURL: import.meta.url,
+  resolver: 'import',
+  caches: 'all',
+});
+const v3 = await import(counterBase);
+assert.strictEqual(v3.count, 3);
+
+const wrapCount2 = queryObjects(ModuleWrap, { format: 'count' });
+// Another +1 (v3); v2 is no longer in loadCache and has no other strong
+// holder, so it MAY have been collected already. v1 (pinned by parent) is
+// still alive. Net growth is bounded by the number of active versions in
+// any live strong reference — typically just the current one plus the
+// parent-pinned original.
+assert.ok(
+  wrapCount2 <= wrapCount1 + 1,
+  `After a second cycle, live ModuleWrap count should grow by at most 1 ` +
+  `(got ${wrapCount2}, was ${wrapCount1})`,
+);
+
+delete globalThis.__module_cache_esm_counter;

test/es-module/test-module-clear-cache-import-cjs-race.mjs

@@ -1,25 +1,94 @@
+// Tests race conditions between clearCache and concurrent dynamic imports
+// of a CJS module loaded via import().
+//
+// Scenarios covered:
+//   A) clearCache fires BEFORE the in-flight import promise settles:
+//        p = import(url); clearCache(url); result = await p
+//      The original import must still succeed with the first module instance.
+//
+//   B) Two concurrent imports (sharing the same in-flight job) with clearCache
+//      between them, then a third import after clearing:
+//        p1 = import(url)          → job1 created, cached
+//        p2 = import(url)          → reuses job1 (same in-flight promise)
+//        clearCache(url)           → removes job1 from cache
+//        p3 = import(url)          → job3 created (fresh execution)
+//        [await all three]
+//      p1 and p2 must resolve to the SAME module instance (they shared job1).
+//      p3 must resolve to a DIFFERENT, freshly-executed module instance.
+//
+//   C) clearCache fires AFTER the import has fully settled and another clear +
+//      import is done serially — basic sanity check that repeated cycles work.
+
 import '../common/index.mjs';
 import assert from 'node:assert';
 import { clearCache } from 'node:module';
 
 const url = new URL('../fixtures/module-cache/cjs-counter.js', import.meta.url);
 
-const importPromise = import(url.href);
+// ── Scenario A: clearCache before in-flight import settles ──────────────────
+
+const p_a = import(url.href);  // in-flight; module not yet resolved
 clearCache(url, {
   parentURL: import.meta.url,
   resolver: 'import',
   caches: 'module',
 });
 
-const first = await importPromise;
-assert.strictEqual(first.default.count, 1);
+const result_a = await p_a;
+// Scenario A: in-flight import must still resolve to the first instance.
+assert.strictEqual(result_a.default.count, 1);
+
+// ── Scenario B: two concurrent imports share a job; clearCache between ──────
+
+// Re-seed for a clean counter baseline.
+clearCache(url, {
+  parentURL: import.meta.url,
+  resolver: 'import',
+  caches: 'module',
+});
+
+delete globalThis.__module_cache_cjs_counter;
+
+// Both p_b1 and p_b2 start before clearCache → they share the same in-flight job.
+const p_b1 = import(url.href);
+const p_b2 = import(url.href);
+
+clearCache(url, {
+  parentURL: import.meta.url,
+  resolver: 'import',
+  caches: 'module',
+});
+
+// p_b3 starts after clearCache → gets a fresh independent job.
+const p_b3 = import(url.href);
+
+const [r_b1, r_b2, r_b3] = await Promise.all([p_b1, p_b2, p_b3]);
+
+// p_b1 and p_b2 shared the same in-flight job → identical module namespace.
+assert.strictEqual(r_b1, r_b2);
+// Scenario B: shared job resolves to the first (re-seeded) instance.
+assert.strictEqual(r_b1.default.count, 1);
+
+// p_b3 was created after clearCache → fresh execution, different instance.
+assert.notStrictEqual(r_b3, r_b1);
+assert.strictEqual(r_b3.default.count, 2);
+
+// ── Scenario C: serial repeated cycles ─────────────────────────────────────
+
+clearCache(url, {
+  parentURL: import.meta.url,
+  resolver: 'import',
+  caches: 'module',
+});
+const r_c1 = await import(url.href);
+assert.strictEqual(r_c1.default.count, 3);
 
 clearCache(url, {
   parentURL: import.meta.url,
   resolver: 'import',
   caches: 'module',
 });
-const second = await import(url.href);
-assert.strictEqual(second.default.count, 2);
+const r_c2 = await import(url.href);
+assert.strictEqual(r_c2.default.count, 4);
 
 delete globalThis.__module_cache_cjs_counter;

test/es-module/test-module-clear-cache-static-import-leak.mjs

@@ -0,0 +1,106 @@
+// Flags: --expose-internals
+// Verifies the V8-level memory-retention behaviour of clearCache when a module
+// is statically imported by a still-live parent.
+//
+// BACKGROUND:
+//   When a parent module P statically imports M (via `import … from './M'`),
+//   V8's module instantiation creates an internal strong reference from P's
+//   compiled module record to M's module record.  This link is permanent for
+//   the lifetime of P.  Clearing M from Node.js's JS-level caches (loadCache /
+//   resolveCache) does NOT sever the V8-internal link; the old ModuleWrap for
+//   M stays alive as long as P is alive.
+//
+//   Consequence:  after `clearCache(M)` + `await import(M)`, TWO module
+//   instances coexist:
+//     - M_old : retained by P's V8-internal link, never re-executed by P
+//     - M_new : created by the fresh import(), seen by all NEW importers
+//
+//   This is a *bounded* leak (one stale instance per cleared module per live
+//   static parent), not an unbounded one.  It is unavoidable given the
+//   ECMA-262 HostLoadImportedModule idempotency requirement.
+//
+//   For purely dynamic imports (no static parent holding them) the old
+//   ModuleWrap IS collectible after clearCache — see the second half of this
+//   test which uses checkIfCollectableByCounting to confirm that.
+
+import '../common/index.mjs';
+
+import assert from 'node:assert';
+import { clearCache, createRequire } from 'node:module';
+import { queryObjects } from 'v8';
+
+// Use createRequire to access CJS-only internals from this ESM file.
+const require = createRequire(import.meta.url);
+const { checkIfCollectableByCounting } = require('../common/gc');
+const { internalBinding } = require('internal/test/binding');
+const { ModuleWrap } = internalBinding('module_wrap');
+
+const counterURL = new URL(
+  '../fixtures/module-cache/esm-counter.mjs',
+  import.meta.url,
+).href;
+
+const parentURL = new URL(
+  '../fixtures/module-cache/esm-static-parent.mjs',
+  import.meta.url,
+).href;
+
+// ── Part 1 : static-parent split-brain ──────────────────────────────────────
+// Load the static parent, which in turn statically imports esm-counter.mjs.
+
+const parent = await import(parentURL);
+assert.strictEqual(parent.count, 1);  // counter runs once
+
+// Snapshot the number of live ModuleWraps before clearing.
+const wrapsBefore = queryObjects(ModuleWrap, { format: 'count' });
+
+// Clear the counter's Node-side caches (does NOT sever V8 static links).
+clearCache(counterURL, {
+  parentURL: import.meta.url,
+  resolver: 'import',
+  caches: 'all',
+});
+
+// Re-import counter: a fresh instance is created and executed.
+const fresh = await import(counterURL);
+assert.strictEqual(fresh.count, 2);   // New execution.
+// Parent still sees the OLD instance via the V8-internal static link —
+// the "split-brain" behaviour.
+assert.strictEqual(parent.count, 1);
+
+// After the fresh import there should be MORE live ModuleWraps than before,
+// because the old instance (held by the parent) was NOT collected.
+const wrapsAfter = queryObjects(ModuleWrap, { format: 'count' });
+assert.ok(
+  wrapsAfter > wrapsBefore,
+  `Expected more live ModuleWraps after re-import (old instance retained ` +
+  `by static parent).  before=${wrapsBefore}, after=${wrapsAfter}`,
+);
+
+// ── Part 2 : dynamic-only modules ARE collectible ───────────────────────────
+// Prove that for purely-dynamic imports (no static parent), cleared modules
+// can be garbage-collected.  This confirms that the static-parent case is the
+// source of the memory retention, not clearCache itself.
+
+const baseURL = new URL(
+  '../fixtures/module-cache/esm-counter.mjs',
+  import.meta.url,
+).href;
+
+const outer = 8;
+const inner = 4;
+
+await checkIfCollectableByCounting(async (i) => {
+  for (let j = 0; j < inner; j++) {
+    const url = `${baseURL}?leak-test=${i}-${j}`;
+    await import(url);
+    clearCache(url, {
+      parentURL: import.meta.url,
+      resolver: 'import',
+      caches: 'all',
+    });
+  }
+  return inner;
+}, ModuleWrap, outer, 50);
+
+delete globalThis.__module_cache_esm_counter;

test/fixtures/module-cache/esm-static-parent.mjs

@@ -0,0 +1,3 @@
+// A parent module that statically imports esm-counter.mjs.
+// Used by tests that verify memory retention of statically-linked modules.
+export { count } from './esm-counter.mjs';