From aecd5ebf49c9c4a8a399d5f56d1705288b84ea36 Mon Sep 17 00:00:00 2001
From: zhangyongsheng <zhangyongsheng@youzan.com>
Date: Sun, 17 Jan 2021 21:18:45 +0800
Subject: [PATCH] module: only set cache when finding module succeeds
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

PR-URL: https://github.com/nodejs/node/pull/36642
Fixes: https://github.com/nodejs/node/issues/36638
Reviewed-By: Bradley Farias <bradley.meck@gmail.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
---
 benchmark/module/module-require.js | 80 ++++++++++++++++++++++++++++++
 lib/internal/modules/cjs/loader.js |  5 +-
 test/parallel/test-module-cache.js | 19 +++++++
 3 files changed, 103 insertions(+), 1 deletion(-)
 create mode 100644 benchmark/module/module-require.js
 create mode 100644 test/parallel/test-module-cache.js

diff --git a/benchmark/module/module-require.js b/benchmark/module/module-require.js
new file mode 100644
index 00000000000000..d5b72c4b03f2a9
--- /dev/null
+++ b/benchmark/module/module-require.js
@@ -0,0 +1,80 @@
+'use strict';
+
+const fs = require('fs');
+const path = require('path');
+const common = require('../common.js');
+const tmpdir = require('../../test/common/tmpdir');
+const benchmarkDirectory = path.join(tmpdir.path, 'nodejs-benchmark-module');
+
+const bench = common.createBenchmark(main, {
+  type: ['.js', '.json', 'dir'],
+  n: [1e4],
+});
+
+function main({ type, n }) {
+  tmpdir.refresh();
+  createEntryPoint(n);
+
+  switch (type) {
+    case '.js':
+      measureJSFile(n);
+      break;
+    case '.json':
+      measureJSONFile(n);
+      break;
+    case 'dir':
+      measureDir(n);
+  }
+
+  tmpdir.refresh();
+}
+
+function measureJSFile(n) {
+  bench.start();
+  for (let i = 0; i < n; i++) {
+    require(`${benchmarkDirectory}/${i}`);
+  }
+  bench.end(n);
+}
+
+function measureJSONFile(n) {
+  bench.start();
+  for (let i = 0; i < n; i++) {
+    require(`${benchmarkDirectory}/json_${i}.json`);
+  }
+  bench.end(n);
+}
+
+function measureDir(n) {
+  bench.start();
+  for (let i = 0; i < n; i++) {
+    require(`${benchmarkDirectory}${i}`);
+  }
+  bench.end(n);
+}
+
+function createEntryPoint(n) {
+  fs.mkdirSync(benchmarkDirectory);
+
+  const JSFileContent = 'module.exports = [];';
+  const JSONFileContent = '[]';
+
+  for (let i = 0; i < n; i++) {
+    // JS file.
+    fs.writeFileSync(`${benchmarkDirectory}/${i}.js`, JSFileContent);
+
+    // JSON file.
+    fs.writeFileSync(`${benchmarkDirectory}/json_${i}.json`, JSONFileContent);
+
+    // Dir.
+    fs.mkdirSync(`${benchmarkDirectory}${i}`);
+    fs.writeFileSync(
+      `${benchmarkDirectory}${i}/package.json`,
+      '{"main": "index.js"}'
+    );
+    fs.writeFileSync(
+      `${benchmarkDirectory}${i}/index.js`,
+      JSFileContent
+    );
+  }
+}
diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js
index e7298a2acd782e..7696a3e1907460 100644
--- a/lib/internal/modules/cjs/loader.js
+++ b/lib/internal/modules/cjs/loader.js
@@ -148,7 +148,10 @@ function stat(filename) {
     if (result !== undefined) return result;
   }
   const result = internalModuleStat(filename);
-  if (statCache !== null) statCache.set(filename, result);
+  if (statCache !== null && result >= 0) {
+    // Only set cache when `internalModuleStat(filename)` succeeds.
+    statCache.set(filename, result);
+  }
   return result;
 }
 
diff --git a/test/parallel/test-module-cache.js b/test/parallel/test-module-cache.js
new file mode 100644
index 00000000000000..0f63829750beb1
--- /dev/null
+++ b/test/parallel/test-module-cache.js
@@ -0,0 +1,19 @@
+'use strict';
+require('../common');
+const assert = require('assert');
+const fs = require('fs');
+const path = require('path');
+const tmpdir = require('../common/tmpdir');
+tmpdir.refresh();
+
+const filePath = path.join(tmpdir.path, 'test-module-cache.json');
+assert.throws(
+  () => require(filePath),
+  { code: 'MODULE_NOT_FOUND' }
+);
+
+fs.writeFileSync(filePath, '[]');
+
+const content = require(filePath);
+assert.strictEqual(Array.isArray(content), true);
+assert.strictEqual(content.length, 0);