Skip to content

Commit 34221a1

Browse files
legendecasmarco-ippolito
authored andcommitted
lib: allow CJS source map cache to be reclaimed
Unifies the CJS and ESM source map cache map with SourceMapCacheMap and allows the CJS cache entries to be queried more efficiently with a source url without iteration on an IterableWeakMap. Add a test to verify that the CJS source map cache entry can be reclaimed. PR-URL: #51711 Backport-PR-URL: #56927 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Refs: #52697
1 parent b86f575 commit 34221a1

16 files changed

+394
-277
lines changed

lib/internal/modules/cjs/loader.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -1375,7 +1375,7 @@ function wrapSafe(filename, content, cjsModuleInstance, codeCache, format) {
13751375
// Cache the source map for the module if present.
13761376
const { sourceMapURL } = script;
13771377
if (sourceMapURL) {
1378-
maybeCacheSourceMap(filename, content, this, false, undefined, sourceMapURL);
1378+
maybeCacheSourceMap(filename, content, cjsModuleInstance, false, undefined, sourceMapURL);
13791379
}
13801380

13811381
return {
@@ -1398,7 +1398,7 @@ function wrapSafe(filename, content, cjsModuleInstance, codeCache, format) {
13981398

13991399
// Cache the source map for the module if present.
14001400
if (result.sourceMapURL) {
1401-
maybeCacheSourceMap(filename, content, this, false, undefined, result.sourceMapURL);
1401+
maybeCacheSourceMap(filename, content, cjsModuleInstance, false, undefined, result.sourceMapURL);
14021402
}
14031403

14041404
return result;

lib/internal/modules/esm/translators.js

+3-4
Original file line numberDiff line numberDiff line change
@@ -178,13 +178,12 @@ translators.set('module', function moduleStrategy(url, source, isMain) {
178178
function loadCJSModule(module, source, url, filename, isMain) {
179179
const compileResult = compileFunctionForCJSLoader(source, filename, isMain, false);
180180

181+
const { function: compiledWrapper, sourceMapURL } = compileResult;
181182
// Cache the source map for the cjs module if present.
182-
if (compileResult.sourceMapURL) {
183-
maybeCacheSourceMap(url, source, null, false, undefined, compileResult.sourceMapURL);
183+
if (sourceMapURL) {
184+
maybeCacheSourceMap(url, source, module, false, undefined, sourceMapURL);
184185
}
185186

186-
const compiledWrapper = compileResult.function;
187-
188187
const cascadedLoader = require('internal/modules/esm/loader').getOrInitializeCascadedLoader();
189188
const __dirname = dirname(filename);
190189
// eslint-disable-next-line func-name-matching,func-style

lib/internal/modules/esm/utils.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -343,7 +343,7 @@ function compileSourceTextModule(url, source, cascadedLoader) {
343343
}
344344
// Cache the source map for the module if present.
345345
if (wrap.sourceMapURL) {
346-
maybeCacheSourceMap(url, source, null, false, undefined, wrap.sourceMapURL);
346+
maybeCacheSourceMap(url, source, wrap, false, undefined, wrap.sourceMapURL);
347347
}
348348
return wrap;
349349
}

lib/internal/source_map/prepare_stack_trace.js

+22-3
Original file line numberDiff line numberDiff line change
@@ -139,9 +139,15 @@ function getOriginalSymbolName(sourceMap, callSite, callerCallSite) {
139139
}
140140
}
141141

142-
// Places a snippet of code from where the exception was originally thrown
143-
// above the stack trace. This logic is modeled after GetErrorSource in
144-
// node_errors.cc.
142+
/**
143+
* Return a snippet of code from where the exception was originally thrown
144+
* above the stack trace. This called from GetErrorSource in node_errors.cc.
145+
* @param {import('internal/source_map/source_map').SourceMap} sourceMap - the source map to be used
146+
* @param {string} originalSourcePath - path or url of the original source
147+
* @param {number} originalLine - line number in the original source
148+
* @param {number} originalColumn - column number in the original source
149+
* @returns {string | undefined} - the exact line in the source content or undefined if file not found
150+
*/
145151
function getErrorSource(
146152
sourceMap,
147153
originalSourcePath,
@@ -179,6 +185,12 @@ function getErrorSource(
179185
return exceptionLine;
180186
}
181187

188+
/**
189+
* Retrieve the original source code from the source map's `sources` list or disk.
190+
* @param {import('internal/source_map/source_map').SourceMap.payload} payload
191+
* @param {string} originalSourcePath - path or url of the original source
192+
* @returns {string | undefined} - the source content or undefined if file not found
193+
*/
182194
function getOriginalSource(payload, originalSourcePath) {
183195
let source;
184196
// payload.sources has been normalized to be an array of absolute urls.
@@ -202,6 +214,13 @@ function getOriginalSource(payload, originalSourcePath) {
202214
return source;
203215
}
204216

217+
/**
218+
* Retrieve exact line in the original source code from the source map's `sources` list or disk.
219+
* @param {string} fileName - actual file name
220+
* @param {number} lineNumber - actual line number
221+
* @param {number} columnNumber - actual column number
222+
* @returns {string | undefined} - the source content or undefined if file not found
223+
*/
205224
function getSourceMapErrorSource(fileName, lineNumber, columnNumber) {
206225
const sm = findSourceMap(fileName);
207226
if (sm === undefined) {

lib/internal/source_map/source_map_cache.js

+89-75
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
const {
44
ArrayPrototypePush,
55
JSONParse,
6-
ObjectKeys,
76
RegExpPrototypeExec,
87
SafeMap,
98
StringPrototypeCodePointAt,
@@ -25,17 +24,15 @@ const {
2524
} = require('internal/errors');
2625
const { getLazy } = require('internal/util');
2726

28-
// Since the CJS module cache is mutable, which leads to memory leaks when
29-
// modules are deleted, we use a WeakMap so that the source map cache will
30-
// be purged automatically:
31-
const getCjsSourceMapCache = getLazy(() => {
32-
const { IterableWeakMap } = require('internal/util/iterable_weak_map');
33-
return new IterableWeakMap();
27+
const getModuleSourceMapCache = getLazy(() => {
28+
const { SourceMapCacheMap } = require('internal/source_map/source_map_cache_map');
29+
return new SourceMapCacheMap();
3430
});
3531

36-
// The esm cache is not mutable, so we can use a Map without memory concerns:
37-
const esmSourceMapCache = new SafeMap();
38-
// The generated sources is not mutable, so we can use a Map without memory concerns:
32+
// The generated source module/script instance is not accessible, so we can use
33+
// a Map without memory concerns. Separate generated source entries with the module
34+
// source entries to avoid overriding the module source entries with arbitrary
35+
// source url magic comments.
3936
const generatedSourceMapCache = new SafeMap();
4037
const kLeadingProtocol = /^\w+:\/\//;
4138
const kSourceMappingURLMagicComment = /\/[*/]#\s+sourceMappingURL=(?<sourceMappingURL>[^\s]+)/g;
@@ -52,6 +49,10 @@ function getSourceMapsEnabled() {
5249
return sourceMapsEnabled;
5350
}
5451

52+
/**
53+
* Enables or disables source maps programmatically.
54+
* @param {boolean} val
55+
*/
5556
function setSourceMapsEnabled(val) {
5657
validateBoolean(val, 'val');
5758

@@ -72,6 +73,14 @@ function setSourceMapsEnabled(val) {
7273
sourceMapsEnabled = val;
7374
}
7475

76+
/**
77+
* Extracts the source url from the content if present. For example
78+
* //# sourceURL=file:///path/to/file
79+
*
80+
* Read more at: https://tc39.es/source-map-spec/#linking-evald-code-to-named-generated-code
81+
* @param {string} content - source content
82+
* @returns {string | null} source url or null if not present
83+
*/
7584
function extractSourceURLMagicComment(content) {
7685
let match;
7786
let matchSourceURL;
@@ -90,6 +99,14 @@ function extractSourceURLMagicComment(content) {
9099
return sourceURL;
91100
}
92101

102+
/**
103+
* Extracts the source map url from the content if present. For example
104+
* //# sourceMappingURL=file:///path/to/file
105+
*
106+
* Read more at: https://tc39.es/source-map-spec/#linking-generated-code
107+
* @param {string} content - source content
108+
* @returns {string | null} source map url or null if not present
109+
*/
93110
function extractSourceMapURLMagicComment(content) {
94111
let match;
95112
let lastMatch;
@@ -104,7 +121,17 @@ function extractSourceMapURLMagicComment(content) {
104121
return lastMatch.groups.sourceMappingURL;
105122
}
106123

107-
function maybeCacheSourceMap(filename, content, cjsModuleInstance, isGeneratedSource, sourceURL, sourceMapURL) {
124+
/**
125+
* Caches the source map if it is present in the content, with the given filename, moduleInstance, and sourceURL.
126+
* @param {string} filename - the actual filename
127+
* @param {string} content - the actual source content
128+
* @param {import('internal/modules/cjs/loader').Module | ModuleWrap} moduleInstance - a module instance that
129+
* associated with the source, once this is reclaimed, the source map entry will be removed from the cache
130+
* @param {boolean} isGeneratedSource - if the source was generated and evaluated with the global eval
131+
* @param {string | undefined} sourceURL - the source url
132+
* @param {string | undefined} sourceMapURL - the source map url
133+
*/
134+
function maybeCacheSourceMap(filename, content, moduleInstance, isGeneratedSource, sourceURL, sourceMapURL) {
108135
const sourceMapsEnabled = getSourceMapsEnabled();
109136
if (!(process.env.NODE_V8_COVERAGE || sourceMapsEnabled)) return;
110137
const { normalizeReferrerURL } = require('internal/modules/helpers');
@@ -130,45 +157,32 @@ function maybeCacheSourceMap(filename, content, cjsModuleInstance, isGeneratedSo
130157
}
131158

132159
const data = dataFromUrl(filename, sourceMapURL);
133-
const url = data ? null : sourceMapURL;
134-
if (cjsModuleInstance) {
135-
getCjsSourceMapCache().set(cjsModuleInstance, {
136-
__proto__: null,
137-
filename,
138-
lineLengths: lineLengths(content),
139-
data,
140-
url,
141-
sourceURL,
142-
});
143-
} else if (isGeneratedSource) {
144-
const entry = {
145-
__proto__: null,
146-
lineLengths: lineLengths(content),
147-
data,
148-
url,
149-
sourceURL,
150-
};
160+
const entry = {
161+
__proto__: null,
162+
lineLengths: lineLengths(content),
163+
data,
164+
// Save the source map url if it is not a data url.
165+
sourceMapURL: data ? null : sourceMapURL,
166+
sourceURL,
167+
};
168+
169+
if (isGeneratedSource) {
151170
generatedSourceMapCache.set(filename, entry);
152171
if (sourceURL) {
153172
generatedSourceMapCache.set(sourceURL, entry);
154173
}
155-
} else {
156-
// If there is no cjsModuleInstance and is not generated source assume we are in a
157-
// "modules/esm" context.
158-
const entry = {
159-
__proto__: null,
160-
lineLengths: lineLengths(content),
161-
data,
162-
url,
163-
sourceURL,
164-
};
165-
esmSourceMapCache.set(filename, entry);
166-
if (sourceURL) {
167-
esmSourceMapCache.set(sourceURL, entry);
168-
}
174+
return;
169175
}
176+
// If it is not a generated source, we assume we are in a "cjs/esm"
177+
// context.
178+
const keys = sourceURL ? [filename, sourceURL] : [filename];
179+
getModuleSourceMapCache().set(keys, entry, moduleInstance);
170180
}
171181

182+
/**
183+
* Caches the source map if it is present in the eval'd source.
184+
* @param {string} content - the eval'd source code
185+
*/
172186
function maybeCacheGeneratedSourceMap(content) {
173187
const sourceMapsEnabled = getSourceMapsEnabled();
174188
if (!(process.env.NODE_V8_COVERAGE || sourceMapsEnabled)) return;
@@ -186,6 +200,14 @@ function maybeCacheGeneratedSourceMap(content) {
186200
}
187201
}
188202

203+
/**
204+
* Resolves source map payload data from the source url and source map url.
205+
* If the source map url is a data url, the data is returned.
206+
* Otherwise the source map url is resolved to a file path and the file is read.
207+
* @param {string} sourceURL - url of the source file
208+
* @param {string} sourceMappingURL - url of the source map
209+
* @returns {object} deserialized source map JSON object
210+
*/
189211
function dataFromUrl(sourceURL, sourceMappingURL) {
190212
try {
191213
const url = new URL(sourceMappingURL);
@@ -227,7 +249,11 @@ function lineLengths(content) {
227249
return output;
228250
}
229251

230-
252+
/**
253+
* Read source map from file.
254+
* @param {string} mapURL - file url of the source map
255+
* @returns {object} deserialized source map JSON object
256+
*/
231257
function sourceMapFromFile(mapURL) {
232258
try {
233259
const fs = require('fs');
@@ -281,56 +307,44 @@ function sourcesToAbsolute(baseURL, data) {
281307
return data;
282308
}
283309

284-
// WARNING: The `sourceMapCacheToObject` and `appendCJSCache` run during
285-
// shutdown. In particular, they also run when Workers are terminated, making
286-
// it important that they do not call out to any user-provided code, including
287-
// built-in prototypes that might have been tampered with.
310+
// WARNING: The `sourceMapCacheToObject` runs during shutdown. In particular,
311+
// it also runs when Workers are terminated, making it important that it does
312+
// not call out to any user-provided code, including built-in prototypes that
313+
// might have been tampered with.
288314

289315
// Get serialized representation of source-map cache, this is used
290316
// to persist a cache of source-maps to disk when NODE_V8_COVERAGE is enabled.
291317
function sourceMapCacheToObject() {
292-
const obj = { __proto__: null };
293-
294-
for (const { 0: k, 1: v } of esmSourceMapCache) {
295-
obj[k] = v;
296-
}
297-
298-
appendCJSCache(obj);
299-
300-
if (ObjectKeys(obj).length === 0) {
318+
const moduleSourceMapCache = getModuleSourceMapCache();
319+
if (moduleSourceMapCache.size === 0) {
301320
return undefined;
302321
}
303-
return obj;
304-
}
305322

306-
function appendCJSCache(obj) {
307-
for (const value of getCjsSourceMapCache()) {
308-
obj[value.filename] = {
323+
const obj = { __proto__: null };
324+
for (const { 0: k, 1: v } of moduleSourceMapCache) {
325+
obj[k] = {
309326
__proto__: null,
310-
lineLengths: value.lineLengths,
311-
data: value.data,
312-
url: value.url,
327+
lineLengths: v.lineLengths,
328+
data: v.data,
329+
url: v.sourceMapURL,
313330
};
314331
}
332+
return obj;
315333
}
316334

335+
/**
336+
* Find a source map for a given actual source URL or path.
337+
* @param {string} sourceURL - actual source URL or path
338+
* @returns {import('internal/source_map/source_map').SourceMap | undefined} a source map or undefined if not found
339+
*/
317340
function findSourceMap(sourceURL) {
318341
if (RegExpPrototypeExec(kLeadingProtocol, sourceURL) === null) {
319342
sourceURL = pathToFileURL(sourceURL).href;
320343
}
321344
if (!SourceMap) {
322345
SourceMap = require('internal/source_map/source_map').SourceMap;
323346
}
324-
let entry = esmSourceMapCache.get(sourceURL) ?? generatedSourceMapCache.get(sourceURL);
325-
if (entry === undefined) {
326-
for (const value of getCjsSourceMapCache()) {
327-
const filename = value.filename;
328-
const cachedSourceURL = value.sourceURL;
329-
if (sourceURL === filename || sourceURL === cachedSourceURL) {
330-
entry = value;
331-
}
332-
}
333-
}
347+
const entry = getModuleSourceMapCache().get(sourceURL) ?? generatedSourceMapCache.get(sourceURL);
334348
if (entry === undefined) {
335349
return undefined;
336350
}

0 commit comments

Comments
 (0)