fs: stop recursive mkdir from retrying ENOENT forever

When mkdir() fails with ENOENT, the recursive algorithm assumes the
parent is missing, creates it and retries. On procfs mkdir() keeps
returning ENOENT although the parent exists, and under a concurrent
rmdir() the parent can disappear again, so the retry loop never
terminates: `fs.mkdirSync('/proc/x', { recursive: true })` spins at
100% CPU.

Retry a path that failed with ENOENT only once and report ENOENT the
second time, like the non-recursive call does.

Fixes: #66268
Signed-off-by: marcopiraccini <marco.piraccini@gmail.com>
PR-URL: #66340
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>

Author: marcopiraccini

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);
+}