Skip to content

fs: stop recursive mkdir from retrying ENOENT forever - #66340

Open
marcopiraccini wants to merge 2 commits into
nodejs:mainfrom
marcopiraccini:fs-mkdirp-enoent-loop
Open

marcopiraccini wants to merge 2 commits into
nodejs:mainfrom
marcopiraccini:fs-mkdirp-enoent-loop

Conversation

@marcopiraccini

@marcopiraccini marcopiraccini commented Sep 27, 2026 •

Copy link
Copy Markdown
Contributor

When mkdir() returns ENOENT, recursive mkdir assumes that the parent directory is missing, creates it, and retries the original path.
This can loop indefinitely when the parent exists but mkdir() continues to return ENOENT, as it does under /proc. The same loop can occur if another process removes a parent directory during the operation.

Track paths that have already returned ENOENT. Retry each path once after handling its parent, then return ENOENT if the retry also fails.

Fixes: #66268

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs involving general changes in the lib/ or src/ directories. needs-ci PRs that need a full CI run. labels Sep 27, 2026
@marcopiraccini
marcopiraccini marked this pull request as ready for review September 27, 2026 07:48
try {
fs.mkdirSync(dir);
} catch (err) {
if (err.code !== 'ENOENT') common.skip(`mkdir under /proc fails with ${err.code}`);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why skip here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because the bug is triggerd when `mklkdir() returns ENOENT.
But you are right, is a useless guard here, removed

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: nodejs#66268
Signed-off-by: marcopiraccini <marco.piraccini@gmail.com>
@codecov

codecov Bot commented Sep 27, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.36%. Comparing base (66f26d3) to head (c4a2e89).
⚠️ Report is 30 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #66340      +/-   ##
==========================================
- Coverage   90.38%   90.36%   -0.02%     
==========================================
  Files         790      792       +2     
  Lines      274497   275400     +903     
  Branches    52557    52781     +224     
==========================================
+ Hits       248100   248875     +775     
- Misses      16879    16951      +72     
- Partials     9518     9574      +56     
Files with missing lines Coverage Δ
src/node_file-inl.h 85.48% <100.00%> (+0.42%) ⬆️
src/node_file.cc 75.57% <100.00%> (+0.38%) ⬆️
src/node_file.h 82.35% <ø> (+3.92%) ⬆️

... and 67 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Signed-off-by: marcopiraccini <marco.piraccini@gmail.com>
Comment on lines +16 to +19
fs.mkdir(dir, { recursive: true }, common.mustCall((err) => {
assert.strictEqual(err.code, expected.code);
assert.strictEqual(err.syscall, expected.syscall);
}));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this could be simplified to

Suggested change
fs.mkdir(dir, { recursive: true }, common.mustCall((err) => {
assert.strictEqual(err.code, expected.code);
assert.strictEqual(err.syscall, expected.syscall);
}));
fs.mkdir(dir, { recursive: true }, common.expectsError(expected));

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs involving general changes in the lib/ or src/ directories. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fs.mkdir with recursive: true retries without a progress bound: spins forever on /proc, livelocks under concurrent rmdir

3 participants