fixup! module: add clearCache for CJS and ESM

Author: anonrig

lib/internal/modules/cjs/loader.js

@@ -2202,6 +2202,14 @@ function deleteModuleFromParents(targetModule) {
   let deleted = false;
   for (let i = 0; i < keys.length; i++) {
     const cachedModule = Module._cache[keys[i]];
+    if (cachedModule?.[kFirstModuleParent] === targetModule) {
+      cachedModule[kFirstModuleParent] = undefined;
+      deleted = true;
+    }
+    if (cachedModule?.[kLastModuleParent] === targetModule) {
+      cachedModule[kLastModuleParent] = undefined;
+      deleted = true;
+    }
     const children = cachedModule?.children;
     if (!ArrayIsArray(children)) {
       continue;
@@ -2344,7 +2352,9 @@ function clearCache(specifier, options = kEmptyObject) {
       if (resolvedPath) {
         resolveDeleted = cascadedLoader.deleteResolveCacheByFilename(resolvedPath) || resolveDeleted;
       }
-      result.module = loadDeleted || resolveDeleted;
+      const { clearCjsCache } = require('internal/modules/esm/translators');
+      const cjsCacheDeleted = clearCjsCache(url);
+      result.module = loadDeleted || resolveDeleted || cjsCacheDeleted;
     } catch (err) {
       if (mode === 'module') {
         throw err;

lib/internal/modules/esm/translators.js

@@ -44,7 +44,7 @@ const {
   loadSourceForCJSWithHooks,
   populateCJSExportsFromESM,
 } = require('internal/modules/cjs/loader');
-const { fileURLToPath, pathToFileURL, URL } = require('internal/url');
+const { fileURLToPath, pathToFileURL, URL, URLParse } = require('internal/url');
 let debug = require('internal/util/debuglog').debuglog('esm', (fn) => {
   debug = fn;
 });
@@ -193,6 +193,64 @@ function loadCJSModule(module, source, url, filename, isMain) {
 // TODO: can we use a weak map instead?
 const cjsCache = new SafeMap();
 
+/**
+ * Resolve a file path for a file URL, stripping search/hash.
+ * @param {string} url
+ * @returns {string|null}
+ */
+function getFilePathFromCjsCacheURL(url) {
+  const parsedURL = URLParse(url);
+  if (!parsedURL) {
+    return null;
+  }
+  if (parsedURL.protocol !== 'file:') {
+    return null;
+  }
+  if (parsedURL.search !== '' || parsedURL.hash !== '') {
+    parsedURL.search = '';
+    parsedURL.hash = '';
+  }
+  try {
+    return fileURLToPath(parsedURL);
+  } catch {
+    return null;
+  }
+}
+
+/**
+ * Remove cjsCache entries for a URL and its file-path variants.
+ * @param {string} url
+ * @returns {boolean} true if any entries were deleted.
+ */
+function clearCjsCache(url) {
+  let deleted = cjsCache.delete(url);
+  const filename = getFilePathFromCjsCacheURL(url);
+  if (!filename) {
+    return deleted;
+  }
+
+  const urls = [];
+  for (const entry of cjsCache) {
+    ArrayPrototypePush(urls, entry[0]);
+  }
+
+  for (let i = 0; i < urls.length; i++) {
+    const cachedURL = urls[i];
+    if (cachedURL === url) {
+      continue;
+    }
+    const cachedFilename = getFilePathFromCjsCacheURL(cachedURL);
+    if (cachedFilename === filename) {
+      cjsCache.delete(cachedURL);
+      deleted = true;
+    }
+  }
+
+  return deleted;
+}
+
+exports.clearCjsCache = clearCjsCache;
+
 /**
  * Creates a ModuleWrap object for a CommonJS module.
  * @param {string} url - The URL of the module.
@@ -323,7 +381,9 @@ translators.set('require-commonjs-typescript', (url, translateContext, parentURL
 
 // This goes through Module._load to accommodate monkey-patchers.
 function loadCJSModuleWithModuleLoad(module, source, url, filename, isMain) {
-  assert(module === CJSModule._cache[filename]);
+  if (CJSModule._cache[filename] !== module) {
+    CJSModule._cache[filename] = module;
+  }
   // If it gets here in the translators, the hooks must have already been invoked
   // in the loader. Skip them in the synthetic module evaluation step.
   wrapModuleLoad(filename, undefined, isMain, kShouldSkipModuleHooks);

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

@@ -0,0 +1,17 @@
+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);
+clearCache(url);
+
+const first = await importPromise;
+assert.strictEqual(first.default.count, 1);
+
+clearCache(url);
+const second = await import(url.href);
+assert.strictEqual(second.default.count, 2);
+
+delete globalThis.__module_cache_cjs_counter;

test/es-module/test-module-clear-cache-wasm-leak.mjs

@@ -0,0 +1,26 @@
+// Flags: --no-warnings
+
+import { mustCall } from '../common/index.mjs';
+import assert from 'node:assert';
+import { clearCache, createRequire } from 'node:module';
+
+const require = createRequire(import.meta.url);
+const { checkIfCollectableByCounting } = require('../common/gc');
+
+const baseUrl = new URL('../fixtures/simple.wasm', import.meta.url);
+
+const outer = 8;
+const inner = 4;
+
+const runIteration = mustCall(async (i) => {
+  for (let j = 0; j < inner; j++) {
+    const url = new URL(baseUrl);
+    url.search = `?v=${i}-${j}`;
+    const mod = await import(url.href);
+    assert.strictEqual(mod.add(1, 2), 3);
+    clearCache(url);
+  }
+  return inner;
+}, outer);
+
+checkIfCollectableByCounting(runIteration, WebAssembly.Instance, outer).then(mustCall());

test/es-module/test-module-clear-cache-wasm.mjs

@@ -0,0 +1,17 @@
+// Flags: --no-warnings
+
+import assert from 'node:assert';
+import { clearCache } from 'node:module';
+
+const url = new URL('../fixtures/simple.wasm', import.meta.url);
+
+const first = await import(url.href);
+assert.strictEqual(first.add(1, 2), 3);
+
+const result = clearCache(url);
+assert.strictEqual(result.commonjs, false);
+assert.strictEqual(result.module, true);
+
+const second = await import(url.href);
+assert.strictEqual(second.add(2, 3), 5);
+assert.notStrictEqual(first, second);

test/fixtures/source-map/cjs-closure-source-map.js

@@ -0,0 +1,25 @@
+// Flags: --expose-internals
+'use strict';
+
+const common = require('../common');
+
+const assert = require('node:assert');
+const path = require('node:path');
+const { pathToFileURL } = require('node:url');
+const { clearCache } = require('node:module');
+const { clearCjsCache } = require('internal/modules/esm/translators');
+
+const fixturePath = path.join(__dirname, '..', 'fixtures', 'module-cache', 'cjs-counter.js');
+const url = pathToFileURL(fixturePath);
+
+(async () => {
+  const first = await import(`${url.href}?v=1`);
+  assert.strictEqual(first.default.count, 1);
+
+  const result = clearCache(url);
+  assert.strictEqual(result.commonjs, true);
+  assert.strictEqual(result.module, true);
+
+  assert.strictEqual(clearCjsCache(`${url.href}?v=1`), false);
+  delete globalThis.__module_cache_cjs_counter;
+})().then(common.mustCall());

test/parallel/test-module-clear-cache-cjs-cache.js

@@ -0,0 +1,23 @@
+'use strict';
+
+const common = require('../common');
+
+const path = require('node:path');
+const Module = require('node:module');
+const { clearCache } = require('node:module');
+const { checkIfCollectableByCounting } = require('../common/gc');
+
+const fixture = path.join(__dirname, '..', 'fixtures', 'module-cache', 'cjs-counter.js');
+
+const outer = 16;
+const inner = 64;
+
+checkIfCollectableByCounting(() => {
+  for (let i = 0; i < inner; i++) {
+    require(fixture);
+    clearCache(fixture, { mode: 'commonjs' });
+  }
+  return inner;
+}, Module, outer).then(common.mustCall(() => {
+  delete globalThis.__module_cache_cjs_counter;
+}));

test/parallel/test-module-clear-cache-leak.js

@@ -0,0 +1,35 @@
+// Flags: --expose-internals
+'use strict';
+
+require('../common');
+
+const assert = require('node:assert');
+const path = require('node:path');
+const { clearCache } = require('node:module');
+const { internalBinding } = require('internal/test/binding');
+
+const {
+  privateSymbols: {
+    module_first_parent_private_symbol,
+    module_last_parent_private_symbol,
+  },
+} = internalBinding('util');
+
+const parentPath = path.join(__dirname, '..', 'fixtures', 'module-cache', 'cjs-parent.js');
+const childPath = path.join(__dirname, '..', 'fixtures', 'module-cache', 'cjs-child.js');
+
+require(parentPath);
+
+const childModule = require.cache[childPath];
+const parentModule = require.cache[parentPath];
+
+assert.strictEqual(childModule[module_first_parent_private_symbol], parentModule);
+assert.strictEqual(childModule[module_last_parent_private_symbol], parentModule);
+
+clearCache(parentPath, { mode: 'commonjs' });
+
+assert.strictEqual(childModule[module_first_parent_private_symbol], undefined);
+assert.strictEqual(childModule[module_last_parent_private_symbol], undefined);
+
+clearCache(childPath, { mode: 'commonjs' });
+delete globalThis.__module_cache_cjs_child_counter;

test/parallel/test-module-clear-cache-parent-links.js

@@ -0,0 +1,29 @@
+// Flags: --enable-source-maps
+'use strict';
+
+require('../common');
+
+const assert = require('node:assert');
+const path = require('node:path');
+const { clearCache } = require('node:module');
+
+const fixture = path.join(
+  __dirname,
+  '..',
+  'fixtures',
+  'source-map',
+  'cjs-closure-source-map.js',
+);
+
+const { crash } = require(fixture);
+
+const result = clearCache(fixture);
+assert.strictEqual(result.commonjs, true);
+assert.strictEqual(result.module, false);
+
+try {
+  crash();
+  assert.fail('Expected crash() to throw');
+} catch (err) {
+  assert.match(err.stack, /cjs-closure-source-map-original\.js/);
+}