From 7dfb853c3377e7f774100ee0aff6f26f83640d13 Mon Sep 17 00:00:00 2001 From: louiellan Date: Sat, 20 Jun 2026 08:49:59 +0800 Subject: [PATCH 1/2] fs: treat `std::errc::permission_denied` as `EPERM` error In the thrown exception, `std::errc::permission_denied` is already treated as an `EPERM` error, but it is not included in one of the omittable errors when retrying the `RmSync` operation. This commit includes it. This also fixes the `retryDelay` calculation on Windows where the `retryDelay` is divided by `1000` but the win32's `Sleep` function takes the argument as an `ms` unit, dividing the supplied `ms` unit further to a much smaller delay. Signed-off-by: louiellan --- src/node_file.cc | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/node_file.cc b/src/node_file.cc index d93f213202ec43..2bbe9fa9a36a31 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -1785,6 +1785,9 @@ static void RmSync(const FunctionCallbackInfo& args) { error == std::errc::too_many_files_open || error == std::errc::too_many_files_open_in_system || error == std::errc::directory_not_empty || + #ifdef _WIN32 + error == std::errc::permission_denied || + #endif error == std::errc::operation_not_permitted); }; @@ -1805,7 +1808,7 @@ static void RmSync(const FunctionCallbackInfo& args) { if (retryDelay > 0) { #ifdef _WIN32 - Sleep(i * retryDelay / 1000); + Sleep(i * retryDelay); #else sleep(i * retryDelay / 1000); #endif From 7cf79838f5f26b596c6316d00d2ce6f48e2df47b Mon Sep 17 00:00:00 2001 From: PickBas Date: Wed, 22 Jul 2026 10:05:29 +0200 Subject: [PATCH 2/2] test: add Windows EPERM retry test for rmSync Fixes: https://github.com/nodejs/node/issues/64016 Signed-off-by: PickBas --- src/node_file.cc | 6 +- test/parallel/test-fs-rmSync-eperm-retries.js | 85 +++++++++++++++++++ 2 files changed, 89 insertions(+), 2 deletions(-) create mode 100644 test/parallel/test-fs-rmSync-eperm-retries.js diff --git a/src/node_file.cc b/src/node_file.cc index 2bbe9fa9a36a31..75cc95515f1739 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -1785,9 +1785,9 @@ static void RmSync(const FunctionCallbackInfo& args) { error == std::errc::too_many_files_open || error == std::errc::too_many_files_open_in_system || error == std::errc::directory_not_empty || - #ifdef _WIN32 +#ifdef _WIN32 error == std::errc::permission_denied || - #endif +#endif error == std::errc::operation_not_permitted); }; @@ -1808,8 +1808,10 @@ static void RmSync(const FunctionCallbackInfo& args) { if (retryDelay > 0) { #ifdef _WIN32 + // No conversion needed: Sleep() takes milliseconds. Sleep(i * retryDelay); #else + // sleep() takes seconds, so convert the millisecond delay. sleep(i * retryDelay / 1000); #endif } diff --git a/test/parallel/test-fs-rmSync-eperm-retries.js b/test/parallel/test-fs-rmSync-eperm-retries.js new file mode 100644 index 00000000000000..55f1d9b3070898 --- /dev/null +++ b/test/parallel/test-fs-rmSync-eperm-retries.js @@ -0,0 +1,85 @@ +'use strict'; + +const common = require('../common'); + +if (!common.isWindows) + common.skip('Windows-specific: EPERM sharing-violation retry in rmSync'); + +const tmpdir = require('../common/tmpdir'); +const assert = require('assert'); +const { once } = require('events'); +const { spawn } = require('child_process'); +const fs = require('fs'); +const path = require('path'); + +tmpdir.refresh(); + +// UV_FS_O_EXLOCK opens with share mode 0, so deletion fails with EPERM +// until this process kills the child. +const lockerScript = ` + const fs = require('fs'); + const UV_FS_O_EXLOCK = 0x10000000; + fs.openSync(process.argv[1], fs.constants.O_RDWR | UV_FS_O_EXLOCK); + process.stdout.write('locked'); + setInterval(() => {}, 60_000); +`; + +async function spawnLocker(file) { + const child = spawn(process.execPath, ['-e', lockerScript, file], + { stdio: ['ignore', 'pipe', 'inherit'] }); + const [data] = await once(child.stdout, 'data'); + assert.strictEqual(data.toString(), 'locked'); + return child; +} + +// Sleep before retry i is i * retryDelay ms, so all retries take at least +// retryDelay * (1 + 2 + ... + maxRetries) ms. +function minRetryTime({ maxRetries, retryDelay }) { + return retryDelay * maxRetries * (maxRetries + 1) / 2; +} + +function timedRmThrowsEPERM(dir, options) { + const start = Date.now(); + assert.throws(() => { + fs.rmSync(dir, { recursive: true, ...options }); + }, { + code: 'EPERM', + name: 'Error', + syscall: 'rm', + }); + return Date.now() - start; +} + +(async () => { + const dir = tmpdir.resolve('rm-eperm-retries'); + const file = path.join(dir, 'locked.txt'); + fs.mkdirSync(dir); + fs.writeFileSync(file, 'hello'); + + const child = await spawnLocker(file); + try { + // Proves the lock is effective: no retries means an immediate EPERM. + timedRmThrowsEPERM(dir, { maxRetries: 0, retryDelay: 0 }); + assert.strictEqual(fs.existsSync(file), true); + + const options = { maxRetries: 4, retryDelay: 100 }; + const expected = minRetryTime(options); // 100+200+300+400 = 1000 ms. + const elapsed = timedRmThrowsEPERM(dir, options); + + // Windows timer granularity may shave a few ms off each Sleep() call. + const slack = 16 * options.maxRetries; + assert.ok(elapsed >= expected - slack, + `rmSync() gave up after ${elapsed}ms; expected it to spend at ` + + `least ~${expected}ms on ${options.maxRetries} retries of ` + + `${options.retryDelay}ms escalating delay`); + + // Catches unit confusion (e.g. seconds vs. milliseconds) in the delay. + assert.ok(elapsed < common.platformTimeout(expected * 10), + `rmSync() gave up after ${elapsed}ms; expected roughly ` + + `${expected}ms for ${options.maxRetries} retries`); + } finally { + child.kill(); + } + await once(child, 'exit'); + assert.strictEqual(fs.existsSync(file), true); +})().then(common.mustCall());