Skip to content

Commit 1fb3404

Browse files
committed
fs: bound the mkdir recursive ENOENT retry loop
mkdir(path, { recursive: true }) creates missing parent directories by retrying whenever mkdir() fails with ENOENT, but nothing tracks whether a retry actually made progress. If the same path keeps failing with ENOENT even after its parent exists (e.g. under procfs, or during a racing directory removal), the retry loops forever at 100% CPU with no way to interrupt a sync call. Track the path last requeued after ENOENT in FSContinuationData and clear it on a successful mkdir(). If the same path is retried again with no progress since, fail with the original error instead of looping. Applies to both MKDirpSync and MKDirpAsync. Fixes: #66268 Signed-off-by: agape1225 <49804691+agape1225@users.noreply.github.com>
1 parent 2b3e3db commit 1fb3404

4 files changed

Lines changed: 86 additions & 0 deletions

File tree

‎src/node_file-inl.h‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,19 @@ void FSContinuationData::MaybeSetFirstPath(const std::string& path) {
2727
}
2828
}
2929

30+
bool FSContinuationData::IsRepeatedEnoentRetry(const std::string& path) const {
31+
return has_last_enoent_retry_path_ && last_enoent_retry_path_ == path;
32+
}
33+
34+
void FSContinuationData::SetLastEnoentRetryPath(const std::string& path) {
35+
last_enoent_retry_path_ = path;
36+
has_last_enoent_retry_path_ = true;
37+
}
38+
39+
void FSContinuationData::ClearLastEnoentRetryPath() {
40+
has_last_enoent_retry_path_ = false;
41+
}
42+
3043
std::string FSContinuationData::PopPath() {
3144
CHECK(!paths_.empty());
3245
std::string path = std::move(paths_.back());

‎src/node_file.cc‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1951,6 +1951,7 @@ int MKDirpSync(uv_loop_t* loop,
19511951
// Note: uv_fs_req_cleanup in terminal paths will be called by
19521952
// ~FSReqWrapSync():
19531953
case 0:
1954+
req_wrap->continuation_data()->ClearLastEnoentRetryPath();
19541955
req_wrap->continuation_data()->MaybeSetFirstPath(next_path);
19551956
if (req_wrap->continuation_data()->paths().empty()) {
19561957
return 0;
@@ -1966,6 +1967,15 @@ int MKDirpSync(uv_loop_t* loop,
19661967
std::string dirname =
19671968
next_path.substr(0, next_path.find_last_of(kPathSeparator));
19681969
if (dirname != next_path) {
1970+
if (req_wrap->continuation_data()->IsRepeatedEnoentRetry(
1971+
next_path)) {
1972+
// Retrying this exact path made no progress last time: the
1973+
// parent exists but mkdir() still can't create this path
1974+
// (e.g. under /proc), or a racing process keeps removing and
1975+
// recreating the parent. Fail instead of looping forever.
1976+
return err;
1977+
}
1978+
req_wrap->continuation_data()->SetLastEnoentRetryPath(next_path);
19691979
req_wrap->continuation_data()->PushPath(std::move(next_path));
19701980
req_wrap->continuation_data()->PushPath(std::move(dirname));
19711981
} else if (req_wrap->continuation_data()->paths().empty()) {
@@ -2022,6 +2032,7 @@ int MKDirpAsync(
20222032
// Note: uv_fs_req_cleanup in terminal paths will be called by
20232033
// FSReqAfterScope::~FSReqAfterScope()
20242034
case 0: {
2035+
req_wrap->continuation_data()->ClearLastEnoentRetryPath();
20252036
if (req_wrap->continuation_data()->paths().empty()) {
20262037
req_wrap->continuation_data()->MaybeSetFirstPath(path);
20272038
req_wrap->continuation_data()->Done(0);
@@ -2047,6 +2058,13 @@ int MKDirpAsync(
20472058
std::string dirname =
20482059
path.substr(0, path.find_last_of(kPathSeparator));
20492060
if (dirname != path) {
2061+
if (req_wrap->continuation_data()->IsRepeatedEnoentRetry(
2062+
path)) {
2063+
// See the matching comment in MKDirpSync().
2064+
req_wrap->continuation_data()->Done(err);
2065+
break;
2066+
}
2067+
req_wrap->continuation_data()->SetLastEnoentRetryPath(path);
20502068
req_wrap->continuation_data()->PushPath(path);
20512069
req_wrap->continuation_data()->PushPath(std::move(dirname));
20522070
} else if (req_wrap->continuation_data()->paths().empty()) {

‎src/node_file.h‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,14 @@ class FSContinuationData : public MemoryRetainer {
114114
inline std::string PopPath();
115115
// Used by mkdirp to track the first path created:
116116
inline void MaybeSetFirstPath(const std::string& path);
117+
// Used by mkdirp to detect and stop an unbounded ENOENT retry loop: a
118+
// filesystem can keep returning ENOENT for a path whose parent already
119+
// exists (e.g. procfs), or a racing process can keep removing/recreating
120+
// a parent directory. Either way, retrying the same path a second time
121+
// with no successful mkdir() in between cannot make progress.
122+
inline bool IsRepeatedEnoentRetry(const std::string& path) const;
123+
inline void SetLastEnoentRetryPath(const std::string& path);
124+
inline void ClearLastEnoentRetryPath();
117125
inline void Done(int result);
118126

119127
int mode() const { return mode_; }
@@ -130,6 +138,8 @@ class FSContinuationData : public MemoryRetainer {
130138
int mode_;
131139
std::vector<std::string> paths_;
132140
std::string first_path_;
141+
std::string last_enoent_retry_path_;
142+
bool has_last_enoent_retry_path_ = false;
133143
};
134144

135145
class FSReqBase : public ReqWrap<uv_fs_t> {
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
5+
if (!common.isLinux)
6+
common.skip('this regression is specific to procfs, which only exists on Linux');
7+
8+
// Regression test for https://github.com/nodejs/node/issues/66268.
9+
//
10+
// mkdir(path, { recursive: true }) walks up the path creating missing
11+
// parent directories whenever mkdir() fails with ENOENT. procfs returns
12+
// ENOENT for names it will never let you create even though the parent
13+
// directory (/proc) already exists, which used to make the walk retry the
14+
// same path forever instead of failing.
15+
16+
const assert = require('assert');
17+
const fs = require('fs');
18+
19+
function unwritableProcPath() {
20+
return `/proc/node-test-mkdirp-${process.pid}-${Date.now()}`;
21+
}
22+
23+
{
24+
const target = unwritableProcPath();
25+
assert.throws(() => {
26+
fs.mkdirSync(target, { recursive: true });
27+
}, { code: 'ENOENT' });
28+
}
29+
30+
{
31+
const target = unwritableProcPath();
32+
fs.mkdir(target, { recursive: true }, common.mustCall((err) => {
33+
assert.strictEqual(err.code, 'ENOENT');
34+
}));
35+
}
36+
37+
{
38+
const target = unwritableProcPath();
39+
fs.promises.mkdir(target, { recursive: true }).then(
40+
common.mustNotCall(),
41+
common.mustCall((err) => {
42+
assert.strictEqual(err.code, 'ENOENT');
43+
}),
44+
);
45+
}

0 commit comments

Comments
 (0)