8000 module: fix stat cache · nodejs/node@d94f4c2 · GitHub
[go: up one dir, main page]
Skip to content

Commit d94f4c2

Browse files
BridgeARaddaleax
BridgeAR
authored and
addaleax
committed
module: fix stat cache
The current caching logic broke by [0] because it used destructuring on the module arguments. Since the exported property is a primi 8000 tive counting it up or down would not have any effect anymore in the module that required that property. The original implementation would cache all stat calls caused during bootstrap. Afterwards it would clear the cache and lazy require calls during runtime would create a new cascading cache for the then loaded modules and clear the cache again. This behavior is now restored. This is difficult to test without exposing a lot of information and therfore the existing tests have been removed (as they could not detect the issue). With the broken implementation it caused each module compilation to reset the cache and therefore minimizing the effect drastically. [0] #19177 PR-URL: #26266 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 2a66cd3 commit d94f4c2

File tree

5 files changed

+15
-58
lines changed

5 files changed

+15
-58
lines changed

‎lib/internal/modules/cjs/helpers.js‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,7 @@ function makeRequireFunction(mod) {
1111
const Module = mod.constructor;
1212

1313
function require(path) {
14-
try {
15-
exports.requireDepth += 1;
16-
return mod.require(path);
17-
} finally {
18-
exports.requireDepth -= 1;
19-
}
14+
return mod.require(path);
2015
}
2116

2217
function resolve(request, options) {
@@ -139,7 +134,6 @@ module.exports = exports = {
139134
builtinLibs,
140135
makeRequireFunction,
141136
normalizeReferrerURL,
142-
requireDepth: 0,
143137
stripBOM,
144138
stripShebang
145139
};

‎lib/internal/modules/cjs/loader.js‎

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@ const { safeGetenv } = internalBinding('credentials');
3737
const {
3838
makeRequireFunction,
3939
normalizeReferrerURL,
40-
requireDepth,
4140
stripBOM,
4241
stripShebang
4342
} = require('internal/modules/cjs/helpers');
@@ -85,18 +84,17 @@ const {
8584

8685
const isWindows = process.platform === 'win32';
8786

87+
let requireDepth = 0;
88+
let statCache = new Map();
8889
function stat(filename) {
8990
filename = path.toNamespacedPath(filename);
90-
const cache = stat.cache;
91-
if (cache !== null) {
92-
const result = cache.get(filename);
93-
if (result !== undefined) return result;
94-
}
95-
const result = internalModuleStat(filename);
96-
if (cache !== null) cache.set(filename, result);
91+
if (statCache === null) statCache = new Map();
92+
let result = statCache.get(filename);
93+
if (result !== undefined) return result;
94+
result = internalModuleStat(filename);
95+
statCache.set(filename, result);
9796
return result;
9897
}
99-
stat.cache = null;
10098

10199
function updateChildren(parent, child, scan) {
102100
var children = parent && parent.children;
@@ -702,7 +700,12 @@ Module.prototype.require = function(id) {
702700
throw new ERR_INVALID_ARG_VALUE('id', id,
703701
'must be a non-empty string');
704702
}
705-
return Module._load(id, this, /* isMain */ false);
703+
requireDepth++;
704+
try {
705+
return Module._load(id, this, /* isMain */ false);
706+
} finally {
707+
requireDepth--;
708+
}
706709
};
707710

708711

@@ -785,8 +788,6 @@ Module.prototype._compile = function(content, filename) {
785788
}
786789
var dirname = path.dirname(filename);
787790
var require = makeRequireFunction(this);
788-
var depth = requireDepth;
789-
if (depth === 0) stat.cache = new Map();
790791
var result;
791792
var exports = this.exports;
792793
var thisValue = exports;
@@ -798,7 +799,7 @@ Module.prototype._compile = function(content, filename) {
798799
result = compiledWrapper.call(thisValue, exports, require, module,
799800
filename, dirname);
800801
}
801-
if (depth === 0) stat.cache = null;
802+
if (requireDepth === 0) statCache = null;
802803
return result;
803804
};
804805

‎test/fixtures/module-require-depth/one.js‎

Lines changed: 0 additions & 11 deletions
This file was deleted.

‎test/fixtures/module-require-depth/two.js‎

Lines changed: 0 additions & 11 deletions
This file was deleted.

‎test/parallel/test-module-require-depth.js‎

Lines changed: 0 additions & 16 deletions
This file was deleted.

0 commit comments

Comments
 (0)
0