fs: stop recursive mkdir from retrying ENOENT forever

src/node_file-inl.h

@@ -6,6 +6,8 @@
 #include "node_file.h"
 #include "req_wrap-inl.h"
 
+#include <algorithm>
+
 namespace node {
 namespace fs {
 
@@ -27,6 +29,15 @@ void FSContinuationData::MaybeSetFirstPath(const std::string& path) {
   }
 }
 
+bool FSContinuationData::ShouldRetryENOENT(const std::string& path) {
+  if (std::find(enoent_paths_.begin(), enoent_paths_.end(), path) !=
+      enoent_paths_.end()) {
+    return false;
+  }
+  enoent_paths_.push_back(path);
+  return true;
+}
+
 std::string FSContinuationData::PopPath() {
   CHECK(!paths_.empty());
   std::string path = std::move(paths_.back());

src/node_file.cc

@@ -219,6 +219,7 @@ typedef void (*uv_fs_callback_t)(uv_fs_t*);
 
 void FSContinuationData::MemoryInfo(MemoryTracker* tracker) const {
   tracker->TrackField("paths", paths_);
+  tracker->TrackField("enoent_paths", enoent_paths_);
 }
 
 FileHandleReadWrap::~FileHandleReadWrap() = default;
@@ -1966,6 +1967,9 @@ int MKDirpSync(uv_loop_t* loop,
           std::string dirname =
               next_path.substr(0, next_path.find_last_of(kPathSeparator));
           if (dirname != next_path) {
+            if (!req_wrap->continuation_data()->ShouldRetryENOENT(next_path)) {
+              return err;
+            }
             req_wrap->continuation_data()->PushPath(std::move(next_path));
             req_wrap->continuation_data()->PushPath(std::move(dirname));
           } else if (req_wrap->continuation_data()->paths().empty()) {
@@ -2047,6 +2051,10 @@ int MKDirpAsync(
               std::string dirname =
                   path.substr(0, path.find_last_of(kPathSeparator));
               if (dirname != path) {
+                if (!req_wrap->continuation_data()->ShouldRetryENOENT(path)) {
+                  req_wrap->continuation_data()->Done(err);
+                  break;
+                }
                 req_wrap->continuation_data()->PushPath(path);
                 req_wrap->continuation_data()->PushPath(std::move(dirname));
               } else if (req_wrap->continuation_data()->paths().empty()) {

src/node_file.h

@@ -114,6 +114,9 @@ class FSContinuationData : public MemoryRetainer {
   inline std::string PopPath();
   // Used by mkdirp to track the first path created:
   inline void MaybeSetFirstPath(const std::string& path);
+  // Used by mkdirp to retry a path that failed with ENOENT only once: its
+  // parent has been created or checked by then, so retrying again cannot help.
+  inline bool ShouldRetryENOENT(const std::string& path);
   inline void Done(int result);
 
   int mode() const { return mode_; }
@@ -129,6 +132,7 @@ class FSContinuationData : public MemoryRetainer {
   uv_fs_t* req_;
   int mode_;
   std::vector<std::string> paths_;
+  std::vector<std::string> enoent_paths_;
   std::string first_path_;
 };
 

test/parallel/test-fs-mkdir-recursive-enoent.js

@@ -0,0 +1,22 @@
+'use strict';
+// Refs: https://github.com/nodejs/node/issues/66268
+// On procfs, mkdir fails with ENOENT although the parent exists. A recursive
+// mkdir must report the error instead of retrying forever.
+const common = require('../common');
+const fs = require('fs');
+
+if (!common.isLinux) common.skip('procfs is Linux only');
+
+const assert = require('assert');
+const dir = `/proc/node-test-${process.pid}`;
+const expected = { code: 'ENOENT', syscall: 'mkdir' };
+
+assert.throws(() => fs.mkdirSync(dir, { recursive: true }), expected);
+
+fs.mkdir(dir, { recursive: true }, common.mustCall((err) => {
+  assert.strictEqual(err.code, expected.code);
+  assert.strictEqual(err.syscall, expected.syscall);
+}));
+
+assert.rejects(fs.promises.mkdir(dir, { recursive: true }), expected)
+  .then(common.mustCall());

test/parallel/test-heapdump-fs-mkdir-recursive.js

@@ -0,0 +1,23 @@
+'use strict';
+
+// This tests heap snapshot integration of a pending recursive mkdir.
+
+const common = require('../common');
+const tmpdir = require('../common/tmpdir');
+const { validateByRetainingPath } = require('../common/heap');
+const assert = require('assert');
+const fs = require('fs');
+
+tmpdir.refresh();
+
+{
+  const nodes = validateByRetainingPath('Node / FSContinuationData', []);
+  assert.strictEqual(nodes.length, 0);
+}
+
+fs.mkdir(tmpdir.resolve('a', 'b'), { recursive: true }, common.mustSucceed());
+
+{
+  const nodes = validateByRetainingPath('Node / FSContinuationData', []);
+  assert.strictEqual(nodes.length, 1);
+}