Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 8d76db8

Browse filesBrowse files
bcoedanielleadams
bcoe
authored and
danielleadams
committed
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>
1 parent 1dd744a commit 8d76db8
Copy full SHA for 8d76db8
Expand file treeCollapse file tree

16 files changed

+281
-62
lines changed
Open diff view settings
Collapse file

‎doc/api/module.md‎

Copy file name to clipboardExpand all lines: doc/api/module.md
+4-9Lines changed: 4 additions & 9 deletions
  • Display the source diff
  • Display the rich diff
Original file line numberDiff line numberDiff line change
@@ -135,26 +135,22 @@ import { findSourceMap, SourceMap } from 'module';
135135
const { findSourceMap, SourceMap } = require('module');
136136
```
137137
138-
### `module.findSourceMap(path[, error])`
138+
<!-- Anchors to make sure old links find a target -->
139+
<a id="module_module_findsourcemap_path_error"></a>
140+
### `module.findSourceMap(path)`
141+
139142
<!-- YAML
140143
added:
141144
- v13.7.0
142145
- v12.17.0
143146
-->
144147
145148
* `path` {string}
146-
* `error` {Error}
147149
* Returns: {module.SourceMap}
148150
149151
`path` is the resolved path for the file for which a corresponding source map
150152
should be fetched.
151153
152-
The `error` instance should be passed as the second parameter to `findSourceMap`
153-
in exceptional flows, such as when an overridden
154-
[`Error.prepareStackTrace(error, trace)`][] is invoked. Modules are not added to
155-
the module cache until they are successfully loaded. In these cases, source maps
156-
are associated with the `error` instance along with the `path`.
157-
158154
### Class: `module.SourceMap`
159155
<!-- YAML
160156
added:
@@ -204,7 +200,6 @@ consists of the following keys:
204200
[ES Modules]: esm.md
205201
[Source map v3 format]: https://sourcemaps.info/spec.html#h.mofvlxcwqzej
206202
[`--enable-source-maps`]: cli.md#cli_enable_source_maps
207-
[`Error.prepareStackTrace(error, trace)`]: https://v8.dev/docs/stack-trace-api#customizing-stack-traces
208203
[`NODE_V8_COVERAGE=dir`]: cli.md#cli_node_v8_coverage_dir
209204
[`SourceMap`]: #module_class_module_sourcemap
210205
[`createRequire()`]: #module_module_createrequire_filename
Collapse file

‎doc/api/modules.md‎

Copy file name to clipboardExpand all lines: doc/api/modules.md
+1-1Lines changed: 1 addition & 1 deletion
  • Display the source diff
  • Display the rich diff
Original file line numberDiff line numberDiff line change
@@ -967,7 +967,7 @@ This section was moved to
967967
[Modules: `module` core module](module.md#module_source_map_v3_support).
968968

969969
<!-- Anchors to make sure old links find a target -->
970-
* <a id="modules_module_findsourcemap_path_error" href="module.html#module_module_findsourcemap_path_error">`module.findSourceMap(path[, error])`</a>
970+
* <a id="modules_module_findsourcemap_path_error" href="module.html#module_module_findsourcemap_path">`module.findSourceMap(path)`</a>
971971
* <a id="modules_class_module_sourcemap" href="module.html#module_class_module_sourcemap">Class: `module.SourceMap`</a>
972972
* <a id="modules_new_sourcemap_payload" href="module.html#module_new_sourcemap_payload">`new SourceMap(payload)`</a>
973973
* <a id="modules_sourcemap_payload" href="module.html#module_sourcemap_payload">`sourceMap.payload`</a>
Collapse file

‎lib/internal/per_context/primordials.js‎

Copy file name to clipboardExpand all lines: lib/internal/per_context/primordials.js
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ function makeSafe(unsafe, safe) {
8686
Object.freeze(safe);
8787
return safe;
8888
}
89+
primordials.makeSafe = makeSafe;
8990

9091
// Subclass the constructors because we need to use their prototype
9192
// methods later.
Collapse file

‎lib/internal/source_map/prepare_stack_trace.js‎

Copy file name to clipboardExpand all lines: lib/internal/source_map/prepare_stack_trace.js
+2-1Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ const prepareStackTrace = (globalThis, error, trace) => {
5757
let str = i !== 0 ? '\n at ' : '';
5858
str = `${str}${t}`;
5959
try {
60-
const sm = findSourceMap(t.getFileName(), error);
60+
const sm = findSourceMap(t.getFileName());
6161
if (sm) {
6262
// Source Map V3 lines/columns use zero-based offsets whereas, in
6363
// stack traces, they start at 1/1.
@@ -119,6 +119,7 @@ function getErrorSource(payload, originalSource, firstLine, firstColumn) {
119119
source = readFileSync(originalSourceNoScheme, 'utf8');
120120
} catch (err) {
121121
debug(err);
122+
return '';
122123
}
123124
}
124125

Collapse file

‎lib/internal/source_map/source_map_cache.js‎

Copy file name to clipboardExpand all lines: lib/internal/source_map/source_map_cache.js
+38-50Lines changed: 38 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,18 @@
11
'use strict';
22

33
const {
4+
ArrayPrototypeMap,
45
JSONParse,
56
ObjectCreate,
67
ObjectKeys,
78
ObjectGetOwnPropertyDescriptor,
89
ObjectPrototypeHasOwnProperty,
910
Map,
1011
MapPrototypeEntries,
11-
WeakMap,
12-
WeakMapPrototypeGet,
12+
RegExpPrototypeTest,
13+
SafeMap,
14+
StringPrototypeMatch,
15+
StringPrototypeSplit,
1316
uncurryThis,
1417
} = primordials;
1518

@@ -27,17 +30,17 @@ let debug = require('internal/util/debuglog').debuglog('source_map', (fn) => {
2730
});
2831
const fs = require('fs');
2932
const { getOptionValue } = require('internal/options');
33+
const { IterableWeakMap } = require('internal/util/iterable_weak_map');
3034
const {
3135
normalizeReferrerURL,
3236
} = require('internal/modules/cjs/helpers');
33-
// For cjs, since Module._cache is exposed to users, we use a WeakMap
34-
// keyed on module, facilitating garbage collection.
35-
const cjsSourceMapCache = new WeakMap();
36-
// The esm cache is not exposed to users, so we can use a Map keyed
37-
// on filenames.
38-
const esmSourceMapCache = new Map();
39-
const { fileURLToPath, URL } = require('url');
40-
let Module;
37+
// Since the CJS module cache is mutable, which leads to memory leaks when
38+
// modules are deleted, we use a WeakMap so that the source map cache will
39+
// be purged automatically:
40+
const cjsSourceMapCache = new IterableWeakMap();
41+
// The esm cache is not mutable, so we can use a Map without memory concerns:
42+
const esmSourceMapCache = new SafeMap();
43+
const { fileURLToPath, pathToFileURL, URL } = require('internal/url');
4144
let SourceMap;
4245

4346
let sourceMapsEnabled;
@@ -70,13 +73,14 @@ function maybeCacheSourceMap(filename, content, cjsModuleInstance) {
7073
debug(err.stack);
7174
return;
7275
}
73-
74-
const match = content.match(/\/[*/]#\s+sourceMappingURL=(?<sourceMappingURL>[^\s]+)/);
76+
const match = StringPrototypeMatch(
77+
content,
78+
/\/[*/]#\s+sourceMappingURL=(?<sourceMappingURL>[^\s]+)/
79+
);
7580
if (match) {
7681
const data = dataFromUrl(filename, match.groups.sourceMappingURL);
7782
const url = data ? null : match.groups.sourceMappingURL;
7883
if (cjsModuleInstance) {
79-
if (!Module) Module = require('internal/modules/cjs/loader').Module;
8084
cjsSourceMapCache.set(cjsModuleInstance, {
8185
filename,
8286
lineLengths: lineLengths(content),
@@ -120,7 +124,7 @@ function lineLengths(content) {
120124
// We purposefully keep \r as part of the line-length calculation, in
121125
// cases where there is a \r\n separator, so that this can be taken into
122126
// account in coverage calculations.
123-
return content.split(/\n|\u2028|\u2029/).map((line) => {
127+
return ArrayPrototypeMap(StringPrototypeSplit(content, /\n|\u2028|\u2029/), (line) => {
124128
return line.length;
125129
});
126130
}
@@ -139,8 +143,8 @@ function sourceMapFromFile(mapURL) {
139143
// data:[<mediatype>][;base64],<data> see:
140144
// https://tools.ietf.org/html/rfc2397#section-2
141145
function sourceMapFromDataUrl(sourceURL, url) {
142-
const [format, data] = url.split(',');
143-
const splitFormat = format.split(';');
146+
const [format, data] = StringPrototypeSplit(url, ',');
147+
const splitFormat = StringPrototypeSplit(format, ';');
144148
const contentType = splitFormat[0];
145149
const base64 = splitFormat[splitFormat.length - 1] === 'base64';
146150
if (contentType === 'application/json') {
@@ -207,48 +211,32 @@ function sourceMapCacheToObject() {
207211
return obj;
208212
}
209213

210-
// Since WeakMap can't be iterated over, we use Module._cache's
211-
// keys to facilitate Source Map serialization.
212-
//
213-
// TODO(bcoe): this means we don't currently serialize source-maps attached
214-
// to error instances, only module instances.
215214
function appendCJSCache(obj) {
216-
if (!Module) return;
217-
const cjsModuleCache = ObjectGetValueSafe(Module, '_cache');
218-
const cjsModules = ObjectKeys(cjsModuleCache);
219-
for (let i = 0; i < cjsModules.length; i++) {
220-
const key = cjsModules[i];
221-
const module = ObjectGetValueSafe(cjsModuleCache, key);
222-
const value = WeakMapPrototypeGet(cjsSourceMapCache, module);
223-
if (value) {
224-
// This is okay because `obj` has a null prototype.
225-
obj[`file://${key}`] = {
226-
lineLengths: ObjectGetValueSafe(value, 'lineLengths'),
227-
data: ObjectGetValueSafe(value, 'data'),
228-
url: ObjectGetValueSafe(value, 'url')
229-
};
230-
}
215+
for (const value of cjsSourceMapCache) {
216+
obj[ObjectGetValueSafe(value, 'filename')] = {
217+
lineLengths: ObjectGetValueSafe(value, 'lineLengths'),
218+
data: ObjectGetValueSafe(value, 'data'),
219+
url: ObjectGetValueSafe(value, 'url')
220+
};
231221
}
232222
}
233223

234-
// Attempt to lookup a source map, which is either attached to a file URI, or
235-
// keyed on an error instance.
236-
// TODO(bcoe): once WeakRefs are available in Node.js, refactor to drop
237-
// requirement of error parameter.
238-
function findSourceMap(uri, error) {
239-
if (!Module) Module = require('internal/modules/cjs/loader').Module;
224+
function findSourceMap(sourceURL) {
225+
if (!RegExpPrototypeTest(/^\w+:\/\//, sourceURL)) {
226+
sourceURL = pathToFileURL(sourceURL).href;
227+
}
240228
if (!SourceMap) {
241229
SourceMap = require('internal/source_map/source_map').SourceMap;
242230
}
243-
let sourceMap = cjsSourceMapCache.get(Module._cache[uri]);
244-
if (!uri.startsWith('file://')) uri = normalizeReferrerURL(uri);
245-
if (sourceMap === undefined) {
246-
sourceMap = esmSourceMapCache.get(uri);
247-
}
231+
let sourceMap = esmSourceMapCache.get(sourceURL);
248232
if (sourceMap === undefined) {
249-
const candidateSourceMap = cjsSourceMapCache.get(error);
250-
if (candidateSourceMap && uri === candidateSourceMap.filename) {
251-
sourceMap = candidateSourceMap;
233+
for (const value of cjsSourceMapCache) {
234+
const filename = ObjectGetValueSafe(value, 'filename');
235+
if (sourceURL === filename) {
236+
sourceMap = {
237+
data: ObjectGetValueSafe(value, 'data')
238+
};
239+
}
252240
}
253241
}
254242
if (sourceMap && sourceMap.data) {
Collapse file

‎lib/internal/util/iterable_weak_map.js‎

Copy file name to clipboard
+86Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
'use strict';
2+
3+
const {
4+
makeSafe,
5+
Object,
6+
SafeSet,
7+
SafeWeakMap,
8+
SymbolIterator,
9+
} = primordials;
10+
11+
// TODO(aduh95): Add FinalizationRegistry to primordials
12+
const SafeFinalizationRegistry = makeSafe(
13+
globalThis.FinalizationRegistry,
14+
class SafeFinalizationRegistry extends globalThis.FinalizationRegistry {}
15+
);
16+
17+
// TODO(aduh95): Add WeakRef to primordials
18+
const SafeWeakRef = makeSafe(
19+
globalThis.WeakRef,
20+
class SafeWeakRef extends globalThis.WeakRef {}
21+
);
22+
23+
// This class is modified from the example code in the WeakRefs specification:
24+
// https://github.com/tc39/proposal-weakrefs
25+
// Licensed under ECMA's MIT-style license, see:
26+
// https://github.com/tc39/ecma262/blob/master/LICENSE.md
27+
class IterableWeakMap {
28+
#weakMap = new SafeWeakMap();
29+
#refSet = new SafeSet();
30+
#finalizationGroup = new SafeFinalizationRegistry(cleanup);
31+
32+
set(key, value) {
33+
const entry = this.#weakMap.get(key);
34+
if (entry) {
35+
// If there's already an entry for the object represented by "key",
36+
// the value can be updated without creating a new WeakRef:
37+
this.#weakMap.set(key, { value, ref: entry.ref });
38+
} else {
39+
const ref = new SafeWeakRef(key);
40+
this.#weakMap.set(key, { value, ref });
41+
this.#refSet.add(ref);
42+
this.#finalizationGroup.register(key, {
43+
set: this.#refSet,
44+
ref
45+
}, ref);
46+
}
47+
}
48+
49+
get(key) {
50+
return this.#weakMap.get(key)?.value;
51+
}
52+
53+
has(key) {
54+
return this.#weakMap.has(key);
55+
}
56+
57+
delete(key) {
58+
const entry = this.#weakMap.get(key);
59+
if (!entry) {
60+
return false;
61+
}
62+
this.#weakMap.delete(key);
63+
this.#refSet.delete(entry.ref);
64+
this.#finalizationGroup.unregister(entry.ref);
65+
return true;
66+
}
67+
68+
*[SymbolIterator]() {
69+
for (const ref of this.#refSet) {
70+
const key = ref.deref();
71+
if (!key) continue;
72+
const { value } = this.#weakMap.get(key);
73+
yield value;
74+
}
75+
}
76+
}
77+
78+
function cleanup({ set, ref }) {
79+
set.delete(ref);
80+
}
81+
82+
Object.freeze(IterableWeakMap.prototype);
83+
84+
module.exports = {
85+
IterableWeakMap,
86+
};
Collapse file

‎node.gyp‎

Copy file name to clipboardExpand all lines: node.gyp
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,7 @@
227227
'lib/internal/util/debuglog.js',
228228
'lib/internal/util/inspect.js',
229229
'lib/internal/util/inspector.js',
230+
'lib/internal/util/iterable_weak_map.js',
230231
'lib/internal/util/types.js',
231232
'lib/internal/http2/core.js',
232233
'lib/internal/http2/compat.js',
Collapse file

‎test/fixtures/source-map/throw-on-require-entry.js‎

Copy file name to clipboardExpand all lines: test/fixtures/source-map/throw-on-require-entry.js
+4Lines changed: 4 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Collapse file

‎test/fixtures/source-map/throw-on-require-entry.js.map‎

Copy file name to clipboardExpand all lines: test/fixtures/source-map/throw-on-require-entry.js.map
+1Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Collapse file

‎test/fixtures/source-map/throw-on-require.js‎

Copy file name to clipboardExpand all lines: test/fixtures/source-map/throw-on-require.js
+15Lines changed: 15 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.