module: refactor to use iterable-weak-map

Using an iterable WeakMap (a data-structure that uses WeakRef and
WeakMap), we are able to: stop relying on Module._cache to
serialize source maps; stop requiring an error object when calling
findSourceMap().
PR-URL: #35915
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

Author: bcoe

doc/api/module.md

@@ -146,26 +146,22 @@ import { findSourceMap, SourceMap } from 'module';
 const { findSourceMap, SourceMap } =require('module');
 ```
 
-### `module.findSourceMap(path[, error])`
+<!-- Anchors to make sure old links find a target -->
+<a id="module_module_findsourcemap_path_error"></a>
+### `module.findSourceMap(path)`
+
 <!-- YAML
 added:
  - v13.7.0
  - v12.17.0
 -->
 
 * `path` {string}
-* `error` {Error}
 * Returns: {module.SourceMap}
 
 `path` is the resolved path for the file for which a corresponding source map
 should be fetched.
 
-The `error` instance should be passed as the second parameter to `findSourceMap`
-in exceptional flows, such as when an overridden
-[`Error.prepareStackTrace(error, trace)`][] is invoked. Modules are not added to
-the module cache until they are successfully loaded. In these cases, source maps
-are associated with the `error` instance along with the `path`.
-
 ### Class: `module.SourceMap`
 <!-- YAML
 added:
@@ -215,7 +211,6 @@ consists of the following keys:
 [ES Modules]: esm.md
 [Source map v3 format]: https://sourcemaps.info/spec.html#h.mofvlxcwqzej
 [`--enable-source-maps`]: cli.md#cli_enable_source_maps
-[`Error.prepareStackTrace(error, trace)`]: https://v8.dev/docs/stack-trace-api#customizing-stack-traces
 [`NODE_V8_COVERAGE=dir`]: cli.md#cli_node_v8_coverage_dir
 [`SourceMap`]: #module_class_module_sourcemap
 [`createRequire()`]: #module_module_createrequire_filename

doc/api/modules.md

@@ -976,7 +976,7 @@ This section was moved to
 [Modules: `module` core module](module.md#module_source_map_v3_support).
 
 <!-- Anchors to make sure old links find a target -->
-* <aid="modules_module_findsourcemap_path_error"href="module.html#module_module_findsourcemap_path_error">`module.findSourceMap(path[, error])`</a>
+* <aid="modules_module_findsourcemap_path_error"href="module.html#module_module_findsourcemap_path">`module.findSourceMap(path)`</a>
 * <aid="modules_class_module_sourcemap"href="module.html#module_class_module_sourcemap">Class: `module.SourceMap`</a>
 * <aid="modules_new_sourcemap_payload"href="module.html#module_new_sourcemap_payload">`new SourceMap(payload)`</a>
 * <aid="modules_sourcemap_payload"href="module.html#module_sourcemap_payload">`sourceMap.payload`</a>

lib/internal/per_context/primordials.js

@@ -86,6 +86,7 @@ function makeSafe(unsafe, safe) {
 Object.freeze(safe);
 returnsafe;
 }
+primordials.makeSafe=makeSafe;
 
 // Subclass the constructors because we need to use their prototype
 // methods later.

lib/internal/source_map/prepare_stack_trace.js

@@ -57,7 +57,7 @@ const prepareStackTrace = (globalThis, error, trace) => {
 letstr=i!==0 ? '\n at ' : '';
 str=`${str}${t}`;
 try{
-constsm=findSourceMap(t.getFileName(),error);
+constsm=findSourceMap(t.getFileName());
 if(sm){
 // Source Map V3 lines/columns use zero-based offsets whereas, in
 // stack traces, they start at 1/1.
@@ -119,6 +119,7 @@ function getErrorSource(payload, originalSource, firstLine, firstColumn) {
 source=readFileSync(originalSourceNoScheme,'utf8');
 }catch(err){
 debug(err);
+return'';
 }
 }
 

lib/internal/source_map/source_map_cache.js

@@ -1,15 +1,18 @@
 'use strict';
 
 const{
+ ArrayPrototypeMap,
  JSONParse,
  ObjectCreate,
  ObjectKeys,
  ObjectGetOwnPropertyDescriptor,
  ObjectPrototypeHasOwnProperty,
  Map,
  MapPrototypeEntries,
- WeakMap,
- WeakMapPrototypeGet,
+ RegExpPrototypeTest,
+ SafeMap,
+ StringPrototypeMatch,
+ StringPrototypeSplit,
  uncurryThis,
 }=primordials;
 
@@ -27,17 +30,17 @@ let debug = require('internal/util/debuglog').debuglog('source_map', (fn) => {
 });
 constfs=require('fs');
 const{ getOptionValue }=require('internal/options');
+const{ IterableWeakMap }=require('internal/util/iterable_weak_map');
 const{
  normalizeReferrerURL,
 }=require('internal/modules/cjs/helpers');
-// For cjs, since Module._cache is exposed to users, we use a WeakMap
-// keyed on module, facilitating garbage collection.
-constcjsSourceMapCache=newWeakMap();
-// The esm cache is not exposed to users, so we can use a Map keyed
-// on filenames.
-constesmSourceMapCache=newMap();
-const{ fileURLToPath,URL}=require('url');
-letModule;
+// 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:
+constcjsSourceMapCache=newIterableWeakMap();
+// The esm cache is not mutable, so we can use a Map without memory concerns:
+constesmSourceMapCache=newSafeMap();
+const{ fileURLToPath, pathToFileURL,URL}=require('internal/url');
 letSourceMap;
 
 letsourceMapsEnabled;
@@ -70,13 +73,14 @@ function maybeCacheSourceMap(filename, content, cjsModuleInstance) {
 debug(err.stack);
 return;
 }
-
-constmatch=content.match(/\/[*/]#\s+sourceMappingURL=(?<sourceMappingURL>[^\s]+)/);
+constmatch=StringPrototypeMatch(
+content,
+/\/[*/]#\s+sourceMappingURL=(?<sourceMappingURL>[^\s]+)/
+);
 if(match){
 constdata=dataFromUrl(filename,match.groups.sourceMappingURL);
 consturl=data ? null : match.groups.sourceMappingURL;
 if(cjsModuleInstance){
-if(!Module)Module=require('internal/modules/cjs/loader').Module;
 cjsSourceMapCache.set(cjsModuleInstance,{
  filename,
 lineLengths: lineLengths(content),
@@ -120,7 +124,7 @@ function lineLengths(content) {
 // We purposefully keep \r as part of the line-length calculation, in
 // cases where there is a \r\n separator, so that this can be taken into
 // account in coverage calculations.
-returncontent.split(/\n|\u2028|\u2029/).map((line)=>{
+returnArrayPrototypeMap(StringPrototypeSplit(content,/\n|\u2028|\u2029/),(line)=>{
 returnline.length;
 });
 }
@@ -139,8 +143,8 @@ function sourceMapFromFile(mapURL) {
 // data:[<mediatype>][;base64],<data> see:
 // https://tools.ietf.org/html/rfc2397#section-2
 functionsourceMapFromDataUrl(sourceURL,url){
-const[format,data]=url.split(',');
-constsplitFormat=format.split(';');
+const[format,data]=StringPrototypeSplit(url,',');
+constsplitFormat=StringPrototypeSplit(format,';');
 constcontentType=splitFormat[0];
 constbase64=splitFormat[splitFormat.length-1]==='base64';
 if(contentType==='application/json'){
@@ -207,48 +211,32 @@ function sourceMapCacheToObject() {
 returnobj;
 }
 
-// Since WeakMap can't be iterated over, we use Module._cache's
-// keys to facilitate Source Map serialization.
-//
-// TODO(bcoe): this means we don't currently serialize source-maps attached
-// to error instances, only module instances.
 functionappendCJSCache(obj){
-if(!Module)return;
-constcjsModuleCache=ObjectGetValueSafe(Module,'_cache');
-constcjsModules=ObjectKeys(cjsModuleCache);
-for(leti=0;i<cjsModules.length;i++){
-constkey=cjsModules[i];
-constmodule=ObjectGetValueSafe(cjsModuleCache,key);
-constvalue=WeakMapPrototypeGet(cjsSourceMapCache,module);
-if(value){
-// This is okay because `obj` has a null prototype.
-obj[`file://${key}`]={
-lineLengths: ObjectGetValueSafe(value,'lineLengths'),
-data: ObjectGetValueSafe(value,'data'),
-url: ObjectGetValueSafe(value,'url')
-};
-}
+for(constvalueofcjsSourceMapCache){
+obj[ObjectGetValueSafe(value,'filename')]={
+lineLengths: ObjectGetValueSafe(value,'lineLengths'),
+data: ObjectGetValueSafe(value,'data'),
+url: ObjectGetValueSafe(value,'url')
+};
 }
 }
 
-// Attempt to lookup a source map, which is either attached to a file URI, or
-// keyed on an error instance.
-// TODO(bcoe): once WeakRefs are available in Node.js, refactor to drop
-// requirement of error parameter.
-functionfindSourceMap(uri,error){
-if(!Module)Module=require('internal/modules/cjs/loader').Module;
+functionfindSourceMap(sourceURL){
+if(!RegExpPrototypeTest(/^\w+:\/\//,sourceURL)){
+sourceURL=pathToFileURL(sourceURL).href;
+}
 if(!SourceMap){
 SourceMap=require('internal/source_map/source_map').SourceMap;
 }
-letsourceMap=cjsSourceMapCache.get(Module._cache[uri]);
-if(!uri.startsWith('file://'))uri=normalizeReferrerURL(uri);
-if(sourceMap===undefined){
-sourceMap=esmSourceMapCache.get(uri);
-}
+letsourceMap=esmSourceMapCache.get(sourceURL);
 if(sourceMap===undefined){
-constcandidateSourceMap=cjsSourceMapCache.get(error);
-if(candidateSourceMap&&uri===candidateSourceMap.filename){
-sourceMap=candidateSourceMap;
+for(constvalueofcjsSourceMapCache){
+constfilename=ObjectGetValueSafe(value,'filename');
+if(sourceURL===filename){
+sourceMap={
+data: ObjectGetValueSafe(value,'data')
+};
+}
 }
 }
 if(sourceMap&&sourceMap.data){

lib/internal/util/iterable_weak_map.js

@@ -0,0 +1,86 @@
+'use strict';
+
+const{
+ makeSafe,
+ Object,
+ SafeSet,
+ SafeWeakMap,
+ SymbolIterator,
+}=primordials;
+
+// TODO(aduh95): Add FinalizationRegistry to primordials
+constSafeFinalizationRegistry=makeSafe(
+globalThis.FinalizationRegistry,
+classSafeFinalizationRegistryextendsglobalThis.FinalizationRegistry{}
+);
+
+// TODO(aduh95): Add WeakRef to primordials
+constSafeWeakRef=makeSafe(
+globalThis.WeakRef,
+classSafeWeakRefextendsglobalThis.WeakRef{}
+);
+
+// This class is modified from the example code in the WeakRefs specification:
+// https://github.com/tc39/proposal-weakrefs
+// Licensed under ECMA's MIT-style license, see:
+// https://github.com/tc39/ecma262/blob/master/LICENSE.md
+classIterableWeakMap{
+ #weakMap =newSafeWeakMap();
+ #refSet =newSafeSet();
+ #finalizationGroup =newSafeFinalizationRegistry(cleanup);
+
+set(key,value){
+constentry=this.#weakMap.get(key);
+if(entry){
+// If there's already an entry for the object represented by "key",
+// the value can be updated without creating a new WeakRef:
+this.#weakMap.set(key,{ value,ref: entry.ref});
+}else{
+constref=newSafeWeakRef(key);
+this.#weakMap.set(key,{ value, ref });
+this.#refSet.add(ref);
+this.#finalizationGroup.register(key,{
+set: this.#refSet,
+ ref
+},ref);
+}
+}
+
+get(key){
+returnthis.#weakMap.get(key)?.value;
+}
+
+has(key){
+returnthis.#weakMap.has(key);
+}
+
+delete(key){
+constentry=this.#weakMap.get(key);
+if(!entry){
+returnfalse;
+}
+this.#weakMap.delete(key);
+this.#refSet.delete(entry.ref);
+this.#finalizationGroup.unregister(entry.ref);
+returntrue;
+}
+
+*[SymbolIterator](){
+for(constrefofthis.#refSet){
+constkey=ref.deref();
+if(!key)continue;
+const{ value }=this.#weakMap.get(key);
+yieldvalue;
+}
+}
+}
+
+functioncleanup({ set, ref }){
+set.delete(ref);
+}
+
+Object.freeze(IterableWeakMap.prototype);
+
+module.exports={
+ IterableWeakMap,
+};

node.gyp

@@ -211,6 +211,7 @@
 'lib/internal/util/debuglog.js',
 'lib/internal/util/inspect.js',
 'lib/internal/util/inspector.js',
+'lib/internal/util/iterable_weak_map.js',
 'lib/internal/util/types.js',
 'lib/internal/http2/core.js',
 'lib/internal/http2/compat.js',