diff --git a/src/node_file-inl.h b/src/node_file-inl.h index ebd97c1c8c52..e8b78d91192c 100644 --- a/src/node_file-inl.h +++ b/src/node_file-inl.h @@ -27,6 +27,19 @@ void FSContinuationData::MaybeSetFirstPath(const std::string& path) { } } +bool FSContinuationData::IsRepeatedEnoentRetry(const std::string& path) const { + return has_last_enoent_retry_path_ && last_enoent_retry_path_ == path; +} + +void FSContinuationData::SetLastEnoentRetryPath(const std::string& path) { + last_enoent_retry_path_ = path; + has_last_enoent_retry_path_ = true; +} + +void FSContinuationData::ClearLastEnoentRetryPath() { + has_last_enoent_retry_path_ = false; +} + 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 a95cd5eba149..29d1d21d6fce 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -1951,6 +1951,7 @@ int MKDirpSync(uv_loop_t* loop, // Note: uv_fs_req_cleanup in terminal paths will be called by // ~FSReqWrapSync(): case 0: + req_wrap->continuation_data()->ClearLastEnoentRetryPath(); req_wrap->continuation_data()->MaybeSetFirstPath(next_path); if (req_wrap->continuation_data()->paths().empty()) { return 0; @@ -1966,6 +1967,15 @@ 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()->IsRepeatedEnoentRetry( + next_path)) { + // Retrying this exact path made no progress last time: the + // parent exists but mkdir() still can't create this path + // (e.g. under /proc), or a racing process keeps removing and + // recreating the parent. Fail instead of looping forever. + return err; + } + req_wrap->continuation_data()->SetLastEnoentRetryPath(next_path); 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()) { @@ -2022,6 +2032,7 @@ int MKDirpAsync( // Note: uv_fs_req_cleanup in terminal paths will be called by // FSReqAfterScope::~FSReqAfterScope() case 0: { + req_wrap->continuation_data()->ClearLastEnoentRetryPath(); if (req_wrap->continuation_data()->paths().empty()) { req_wrap->continuation_data()->MaybeSetFirstPath(path); req_wrap->continuation_data()->Done(0); @@ -2047,6 +2058,13 @@ int MKDirpAsync( std::string dirname = path.substr(0, path.find_last_of(kPathSeparator)); if (dirname != path) { + if (req_wrap->continuation_data()->IsRepeatedEnoentRetry( + path)) { + // See the matching comment in MKDirpSync(). + req_wrap->continuation_data()->Done(err); + break; + } + req_wrap->continuation_data()->SetLastEnoentRetryPath(path); 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..96ef7c825d13 100644 --- a/src/node_file.h +++ b/src/node_file.h @@ -114,6 +114,14 @@ 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 detect and stop an unbounded ENOENT retry loop: a + // filesystem can keep returning ENOENT for a path whose parent already + // exists (e.g. procfs), or a racing process can keep removing/recreating + // a parent directory. Either way, retrying the same path a second time + // with no successful mkdir() in between cannot make progress. + inline bool IsRepeatedEnoentRetry(const std::string& path) const; + inline void SetLastEnoentRetryPath(const std::string& path); + inline void ClearLastEnoentRetryPath(); inline void Done(int result); int mode() const { return mode_; } @@ -130,6 +138,8 @@ class FSContinuationData : public MemoryRetainer { int mode_; std::vector paths_; std::string first_path_; + std::string last_enoent_retry_path_; + bool has_last_enoent_retry_path_ = false; }; class FSReqBase : public ReqWrap { diff --git a/test/parallel/test-fs-mkdir-recursive-proc-enoent.js b/test/parallel/test-fs-mkdir-recursive-proc-enoent.js new file mode 100644 index 000000000000..3cb9ea7dd63b --- /dev/null +++ b/test/parallel/test-fs-mkdir-recursive-proc-enoent.js @@ -0,0 +1,43 @@ +'use strict'; + +const common = require('../common'); + +if (!common.isLinux) + common.skip('this regression is specific to procfs, which only exists on Linux'); + +// Regression test for https://github.com/nodejs/node/issues/66268. +// +// mkdir(path, { recursive: true }) walks up the path creating missing +// parent directories whenever mkdir() fails with ENOENT. procfs returns +// ENOENT for names it will never let you create even though the parent +// directory (/proc) already exists, which used to make the walk retry the +// same path forever instead of failing. + +const assert = require('assert'); +const fs = require('fs'); + +function unwritableProcPath() { + return `/proc/node-test-mkdirp-${process.pid}-${Date.now()}`; +} + +{ + const target = unwritableProcPath(); + assert.throws(() => { + fs.mkdirSync(target, { recursive: true }); + }, { code: 'ENOENT' }); +} + +{ + const target = unwritableProcPath(); + fs.mkdir(target, { recursive: true }, common.mustCall((err) => { + assert.strictEqual(err.code, 'ENOENT'); + })); +} + +{ + const target = unwritableProcPath(); + assert.rejects( + fs.promises.mkdir(target, { recursive: true }), + { code: 'ENOENT' }, + ).then(common.mustCall()); +}