lib: allow CJS source map cache to be reclaimed

Unifies the CJS and ESM source map cache map and allows the CJS cache
entries to be queried more efficiently with a source url without
iteration on an IterableWeakMap. Reclaims the CJS cache entry with
a FinalizationRegistry instead.

Add a test to verify that the CJS source map cache entry can be
reclaimed.

Author: legendecas

lib/internal/modules/cjs/loader.js

@@ -1271,7 +1271,7 @@ function wrapSafe(filename, content, cjsModuleInstance, codeCache) {
 
     // Cache the source map for the module if present.
     if (script.sourceMapURL) {
-      maybeCacheSourceMap(filename, content, this, false, undefined, script.sourceMapURL);
+      maybeCacheSourceMap(filename, content, cjsModuleInstance, false, undefined, script.sourceMapURL);
     }
 
     return runScriptInThisContext(script, true, false);
@@ -1302,7 +1302,7 @@ function wrapSafe(filename, content, cjsModuleInstance, codeCache) {
 
     // Cache the source map for the module if present.
     if (result.sourceMapURL) {
-      maybeCacheSourceMap(filename, content, this, false, undefined, result.sourceMapURL);
+      maybeCacheSourceMap(filename, content, cjsModuleInstance, false, undefined, result.sourceMapURL);
     }
 
     return result.function;

lib/internal/source_map/prepare_stack_trace.js

@@ -114,9 +114,15 @@ function getOriginalSymbolName(sourceMap, trace, curIndex) {
   }
 }
 
-// Places a snippet of code from where the exception was originally thrown
-// above the stack trace. This logic is modeled after GetErrorSource in
-// node_errors.cc.
+/**
+ * Return a snippet of code from where the exception was originally thrown
+ * above the stack trace. This called from GetErrorSource in node_errors.cc.
+ * @param {import('internal/source_map/source_map').SourceMap} sourceMap - the source map to be used
+ * @param {string} originalSourcePath - path or url of the original source
+ * @param {number} originalLine - line number in the original source
+ * @param {number} originalColumn - column number in the original source
+ * @returns {string | undefined} - the exact line in the source content or undefined if file not found
+ */
 function getErrorSource(
   sourceMap,
   originalSourcePath,
@@ -154,6 +160,12 @@ function getErrorSource(
   return exceptionLine;
 }
 
+/**
+ * Retrieve the original source code from the source map's `sources` list or disk.
+ * @param {import('internal/source_map/source_map').SourceMap.payload} payload
+ * @param {string} originalSourcePath - path or url of the original source
+ * @returns {string | undefined} - the source content or undefined if file not found
+ */
 function getOriginalSource(payload, originalSourcePath) {
   let source;
   // payload.sources has been normalized to be an array of absolute urls.
@@ -177,6 +189,13 @@ function getOriginalSource(payload, originalSourcePath) {
   return source;
 }
 
+/**
+ * Retrieve exact line in the original source code from the source map's `sources` list or disk.
+ * @param {string} fileName - actual file name
+ * @param {number} lineNumber - actual line number
+ * @param {number} columnNumber - actual column number
+ * @returns {string | undefined} - the source content or undefined if file not found
+ */
 function getSourceMapErrorSource(fileName, lineNumber, columnNumber) {
   const sm = findSourceMap(fileName);
   if (sm === undefined) {

lib/internal/source_map/source_map_cache.js

@@ -3,9 +3,10 @@
 const {
   ArrayPrototypePush,
   JSONParse,
-  ObjectKeys,
   RegExpPrototypeExec,
+  SafeFinalizationRegistry,
   SafeMap,
+  SafeWeakRef,
   StringPrototypeCodePointAt,
   StringPrototypeSplit,
 } = primordials;
@@ -23,19 +24,26 @@ const {
 const {
   setInternalPrepareStackTrace,
 } = require('internal/errors');
-const { getLazy } = require('internal/util');
-
-// Since the CJS module cache is mutable, which leads to memory leaks when
-// modules are deleted, we use a WeakMap so that the source map cache will
-// be purged automatically:
-const getCjsSourceMapCache = getLazy(() => {
-  const { IterableWeakMap } = require('internal/util/iterable_weak_map');
-  return new IterableWeakMap();
-});
 
-// The esm cache is not mutable, so we can use a Map without memory concerns:
-const esmSourceMapCache = new SafeMap();
-// The generated sources is not mutable, so we can use a Map without memory concerns:
+// The cached module instance can be removed from the global module registry
+// with approaches like mutating `require.cache`.
+// The `moduleSourceMapCache` exposes entries by `filename` and `sourceURL`.
+// In the case of mutated module registry, obsolete entries are removed from
+// the cache by the `moduleFinalizationRegistry`.
+const moduleSourceMapCache = new SafeMap();
+const moduleFinalizationRegistry = new SafeFinalizationRegistry((key) => {
+  const instanceRef = moduleSourceMapCache.get(key).moduleInstanceRef;
+  // Delete the entry if the instanceRef has been reclaimed.
+  // If the instanceRef is not reclaimed, the entry was overridden by a new
+  // module instance.
+  if (instanceRef && instanceRef.deref() === undefined) {
+    moduleSourceMapCache.delete(key);
+  }
+});
+// The generated source module/script instance is not accessible, so we can use
+// a Map without memory concerns. Separate generated source entries with the module
+// source entries to avoid overriding the module source entries with arbitrary
+// source url magic comments.
 const generatedSourceMapCache = new SafeMap();
 const kLeadingProtocol = /^\w+:\/\//;
 const kSourceMappingURLMagicComment = /\/[*/]#\s+sourceMappingURL=(?<sourceMappingURL>[^\s]+)/g;
@@ -52,6 +60,10 @@ function getSourceMapsEnabled() {
   return sourceMapsEnabled;
 }
 
+/**
+ * Enables or disables source maps programmatically.
+ * @param {boolean} val
+ */
 function setSourceMapsEnabled(val) {
   validateBoolean(val, 'val');
 
@@ -72,6 +84,14 @@ function setSourceMapsEnabled(val) {
   sourceMapsEnabled = val;
 }
 
+/**
+ * Extracts the source url from the content if present. For example
+ * //# sourceURL=file:///path/to/file
+ *
+ * Read more at: https://tc39.es/source-map-spec/#linking-evald-code-to-named-generated-code
+ * @param {string} content - source content
+ * @returns {string | null} source url or null if not present
+ */
 function extractSourceURLMagicComment(content) {
   let match;
   let matchSourceURL;
@@ -90,6 +110,14 @@ function extractSourceURLMagicComment(content) {
   return sourceURL;
 }
 
+/**
+ * Extracts the source map url from the content if present. For example
+ * //# sourceMappingURL=file:///path/to/file
+ *
+ * Read more at: https://tc39.es/source-map-spec/#linking-generated-code
+ * @param {string} content - source content
+ * @returns {string | null} source map url or null if not present
+ */
 function extractSourceMapURLMagicComment(content) {
   let match;
   let lastMatch;
@@ -104,7 +132,17 @@ function extractSourceMapURLMagicComment(content) {
   return lastMatch.groups.sourceMappingURL;
 }
 
-function maybeCacheSourceMap(filename, content, cjsModuleInstance, isGeneratedSource, sourceURL, sourceMapURL) {
+/**
+ * Caches the source map if it is present in the content, with the given filename, moduleInstance, and sourceURL.
+ * @param {string} filename - the actual filename
+ * @param {string} content - the actual source content
+ * @param {import('internal/modules/cjs/loader').Module | undefined} moduleInstance - a CJS module instance that
+ * associated with the source, once this is reclaimed, the source map entry will be removed from the cache
+ * @param {boolean} isGeneratedSource - if the source was generated and evaluated with the global eval
+ * @param {string | undefined} sourceURL - the source url
+ * @param {string | undefined} sourceMapURL - the source map url
+ */
+function maybeCacheSourceMap(filename, content, moduleInstance, isGeneratedSource, sourceURL, sourceMapURL) {
   const sourceMapsEnabled = getSourceMapsEnabled();
   if (!(process.env.NODE_V8_COVERAGE || sourceMapsEnabled)) return;
   const { normalizeReferrerURL } = require('internal/modules/helpers');
@@ -129,16 +167,7 @@ function maybeCacheSourceMap(filename, content, cjsModuleInstance, isGeneratedSo
 
   const data = dataFromUrl(filename, sourceMapURL);
   const url = data ? null : sourceMapURL;
-  if (cjsModuleInstance) {
-    getCjsSourceMapCache().set(cjsModuleInstance, {
-      __proto__: null,
-      filename,
-      lineLengths: lineLengths(content),
-      data,
-      url,
-      sourceURL,
-    });
-  } else if (isGeneratedSource) {
+  if (isGeneratedSource) {
     const entry = {
       __proto__: null,
       lineLengths: lineLengths(content),
@@ -150,23 +179,49 @@ function maybeCacheSourceMap(filename, content, cjsModuleInstance, isGeneratedSo
     if (sourceURL) {
       generatedSourceMapCache.set(sourceURL, entry);
     }
-  } else {
-    // If there is no cjsModuleInstance and is not generated source assume we are in a
-    // "modules/esm" context.
-    const entry = {
-      __proto__: null,
-      lineLengths: lineLengths(content),
-      data,
-      url,
-      sourceURL,
-    };
-    esmSourceMapCache.set(filename, entry);
-    if (sourceURL) {
-      esmSourceMapCache.set(sourceURL, entry);
-    }
+    return;
+  }
+
+  // If it is not a generated source, we assume we are in a "cjs/esm"
+  // context.
+  const entry = {
+    __proto__: null,
+    lineLengths: lineLengths(content),
+    data,
+    url,
+    sourceURL,
+    moduleInstanceRef: moduleInstance ? new SafeWeakRef(moduleInstance) : undefined,
+  };
+  setModuleSourceMapCache(filename, sourceURL, moduleInstance, entry);
+}
+
+/**
+ * Registers the module entry in the `moduleSourceMapCache` with the filename and sourceURL.
+ * @param {string} filename - the actual filename, in the form of a file URL
+ * @param {string | undefined} sourceURL - the source url
+ * @param {import('internal/modules/cjs/loader').Module | undefined} moduleInstance - a CJS module instance
+ * @param {object} entry - the source map cache entry
+ */
+function setModuleSourceMapCache(filename, sourceURL, moduleInstance, entry) {
+  moduleSourceMapCache.set(filename, entry);
+  if (sourceURL) {
+    moduleSourceMapCache.set(sourceURL, entry);
+  }
+  // Skip if the module instance is not present.
+  if (moduleInstance == null) return;
+
+  // Register the module instance with the finalization registry to clear the
+  // entry once the module instance is been reclaimed.
+  moduleFinalizationRegistry.register(moduleInstance, filename);
+  if (sourceURL) {
+    moduleFinalizationRegistry.register(moduleInstance, sourceURL);
   }
 }
 
+/**
+ * Caches the source map if it is present in the eval'd source.
+ * @param {string} content - the eval'd source code
+ */
 function maybeCacheGeneratedSourceMap(content) {
   const sourceMapsEnabled = getSourceMapsEnabled();
   if (!(process.env.NODE_V8_COVERAGE || sourceMapsEnabled)) return;
@@ -184,6 +239,14 @@ function maybeCacheGeneratedSourceMap(content) {
   }
 }
 
+/**
+ * Resolves source map payload data from the source url and source map url.
+ * If the source map url is a data url, the data is returned.
+ * Otherwise the source map url is resolved to a file path and the file is read.
+ * @param {string} sourceURL - url of the source file
+ * @param {string} sourceMappingURL - url of the source map
+ * @returns {object} deserialized source map JSON object
+ */
 function dataFromUrl(sourceURL, sourceMappingURL) {
   try {
     const url = new URL(sourceMappingURL);
@@ -225,7 +288,11 @@ function lineLengths(content) {
   return output;
 }
 
-
+/**
+ * Read source map from file.
+ * @param {string} mapURL - file url of the source map
+ * @returns {object} deserialized source map JSON object
+ */
 function sourceMapFromFile(mapURL) {
   try {
     const fs = require('fs');
@@ -279,56 +346,43 @@ function sourcesToAbsolute(baseURL, data) {
   return data;
 }
 
-// WARNING: The `sourceMapCacheToObject` and `appendCJSCache` run during
-// shutdown. In particular, they also run when Workers are terminated, making
-// it important that they do not call out to any user-provided code, including
-// built-in prototypes that might have been tampered with.
+// WARNING: The `sourceMapCacheToObject` run during shutdown. In particular,
+// they also run when Workers are terminated, making it important that they do
+// not call out to any user-provided code, including built-in prototypes that
+// might have been tampered with.
 
 // Get serialized representation of source-map cache, this is used
 // to persist a cache of source-maps to disk when NODE_V8_COVERAGE is enabled.
 function sourceMapCacheToObject() {
-  const obj = { __proto__: null };
-
-  for (const { 0: k, 1: v } of esmSourceMapCache) {
-    obj[k] = v;
-  }
-
-  appendCJSCache(obj);
-
-  if (ObjectKeys(obj).length === 0) {
+  if (moduleSourceMapCache.size === 0) {
     return undefined;
   }
-  return obj;
-}
 
-function appendCJSCache(obj) {
-  for (const value of getCjsSourceMapCache()) {
-    obj[value.filename] = {
+  const obj = { __proto__: null };
+  for (const { 0: k, 1: v } of moduleSourceMapCache) {
+    obj[k] = {
       __proto__: null,
-      lineLengths: value.lineLengths,
-      data: value.data,
-      url: value.url,
+      lineLengths: v.lineLengths,
+      data: v.data,
+      url: v.url,
     };
   }
+  return obj;
 }
 
+/**
+ * Find a source map for a given actual source URL or path.
+ * @param {string} sourceURL - actual source URL or path
+ * @returns {import('internal/source_map/source_map').SourceMap | undefined} a source map or undefined if not found
+ */
 function findSourceMap(sourceURL) {
   if (RegExpPrototypeExec(kLeadingProtocol, sourceURL) === null) {
     sourceURL = pathToFileURL(sourceURL).href;
   }
   if (!SourceMap) {
     SourceMap = require('internal/source_map/source_map').SourceMap;
   }
-  let entry = esmSourceMapCache.get(sourceURL) ?? generatedSourceMapCache.get(sourceURL);
-  if (entry === undefined) {
-    for (const value of getCjsSourceMapCache()) {
-      const filename = value.filename;
-      const cachedSourceURL = value.sourceURL;
-      if (sourceURL === filename || sourceURL === cachedSourceURL) {
-        entry = value;
-      }
-    }
-  }
+  const entry = moduleSourceMapCache.get(sourceURL) ?? generatedSourceMapCache.get(sourceURL);
   if (entry === undefined) {
     return undefined;
   }