diff --git a/src/node_file-inl.h b/src/node_file-inl.h index ebd97c1c8c52..803446b1efba 100644 --- a/src/node_file-inl.h +++ b/src/node_file-inl.h @@ -6,6 +6,8 @@ #include "node_file.h" #include "req_wrap-inl.h" +#include + 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()); diff --git a/src/node_file.cc b/src/node_file.cc index b624e9da41bd..130691aca8db 100644 --- a/src/node_file.cc +++ b/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()) { diff --git a/src/node_file.h b/src/node_file.h index fab01a4c17b8..1e88f79d6fa8 100644 --- a/src/node_file.h +++ b/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 paths_; + std::vector enoent_paths_; std::string first_path_; }; diff --git a/test/parallel/test-fs-mkdir-recursive-enoent.js b/test/parallel/test-fs-mkdir-recursive-enoent.js new file mode 100644 index 000000000000..91e1c57ababe --- /dev/null +++ b/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()); diff --git a/test/parallel/test-heapdump-fs-mkdir-recursive.js b/test/parallel/test-heapdump-fs-mkdir-recursive.js new file mode 100644 index 000000000000..4da402079437 --- /dev/null +++ b/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); +}