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 <[email protected]>
PR-URL: #64698
Fixes: #64016
Refs: #64017
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Stefan Stojanovic <[email protected]>

Author: PickBas

src/node_file.cc

@@ -1785,6 +1785,9 @@ static void RmSync(const FunctionCallbackInfo<Value>& 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,8 +1808,10 @@ static void RmSync(const FunctionCallbackInfo<Value>& args) {
 
     if (retryDelay > 0) {
 #ifdef _WIN32
-      Sleep(i * retryDelay / 1000);
+      // No conversion needed: Sleep() takes milliseconds.
+      Sleep(i * retryDelay);
 #else
+      // sleep() takes seconds, so convert the millisecond delay.
       sleep(i * retryDelay / 1000);
 #endif
     }

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());