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 primitive 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
nodejs · BridgeAR · Dec 31, 2018 · Feb 22, 2019 · Feb 22, 2019 · Feb 28, 2019
commit 0b3c65f366ad95971ba550e194443bfc4b7ef269
diff --git a/lib/internal/modules/cjs/helpers.js b/lib/internal/modules/cjs/helpers.js
@@ -11,12 +11,7 @@ function makeRequireFunction(mod) {
   const Module = mod.constructor;
 
   function require(path) {
-    try {
-      exports.requireDepth += 1;
-      return mod.require(path);
-    } finally {
-      exports.requireDepth -= 1;
-    }
+    return mod.require(path);
   }
 
   function resolve(request, options) {
@@ -139,7 +134,6 @@ module.exports = exports = {
   builtinLibs,
   makeRequireFunction,
   normalizeReferrerURL,
-  requireDepth: 0,
   stripBOM,
   stripShebang
 };
diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js
@@ -37,7 +37,6 @@ const { safeGetenv } = internalBinding('credentials');
 const {
   makeRequireFunction,
   normalizeReferrerURL,
-  requireDepth,
   stripBOM,
   stripShebang
 } = require('internal/modules/cjs/helpers');
@@ -85,18 +84,17 @@ const {
 
 const isWindows = process.platform === 'win32';
 
+let requireDepth = 0;
+let statCache = new Map();
 function stat(filename) {
   filename = path.toNamespacedPath(filename);
-  const cache = stat.cache;
-  if (cache !== null) {
-    const result = cache.get(filename);
-    if (result !== undefined) return result;
-  }
-  const result = internalModuleStat(filename);
-  if (cache !== null) cache.set(filename, result);
+  if (statCache === null) statCache = new Map();
+  let result = statCache.get(filename);
+  if (result !== undefined) return result;
+  result = internalModuleStat(filename);
+  statCache.set(filename, result);
   return result;
 }
-stat.cache = null;
 
 function updateChildren(parent, child, scan) {
   var children = parent && parent.children;
@@ -710,7 +708,12 @@ Module.prototype.require = function(id) {
     throw new ERR_INVALID_ARG_VALUE('id', id,
                                     'must be a non-empty string');
   }
-  return Module._load(id, this, /* isMain */ false);
+  requireDepth++;
+  try {
+    return Module._load(id, this, /* isMain */ false);
+  } finally {
+    requireDepth--;
+  }
 };
 
 
@@ -793,8 +796,6 @@ Module.prototype._compile = function(content, filename) {
   }
   var dirname = path.dirname(filename);
   var require = makeRequireFunction(this);
-  var depth = requireDepth;
-  if (depth === 0) stat.cache = new Map();
   var result;
   var exports = this.exports;
   var thisValue = exports;
@@ -806,7 +807,7 @@ Module.prototype._compile = function(content, filename) {
     result = compiledWrapper.call(thisValue, exports, require, module,
                                   filename, dirname);
   }
-  if (depth === 0) stat.cache = null;
+  if (requireDepth === 0) statCache = null;
   return result;
 };
 

diff --git a/test/fixtures/module-require-depth/one.js b/test/fixtures/module-require-depth/one.js
diff --git a/test/fixtures/module-require-depth/two.js b/test/fixtures/module-require-depth/two.js
diff --git a/test/parallel/test-module-require-depth.js b/test/parallel/test-module-require-depth.js