fs: refactor unlinksync logic in rimraf

_unlinksync.csv

@@ -0,0 +1 @@
+"binary","filename","configuration","rate","time"

benchmark/fs/bench-rimrafUnlinkSync.js

@@ -0,0 +1,53 @@
+'use strict';
+
+const assert = require('assert');
+const common = require('../common');
+const fs = require('fs');
+const tmpdir = require('../../test/common/tmpdir');
+tmpdir.refresh();
+
+const bench = common.createBenchmark(main, {
+  type: ['existing', 'non-existing'],
+  n: [1e3],
+});
+
+function main({ n, type }) {
+  switch (type) {
+    case 'existing': {
+      for (let i = 0; i < n; i++) {
+        fs.writeFileSync(tmpdir.resolve(`rimrafunlinksync-bench-dir-${i}`), '');
+      }
+
+      bench.start();
+      for (let i = 0; i < n; i++) {
+        try {
+          fs.rmSync(tmpdir.resolve(`rimrafunlinksync-bench-dir-${i}`), {
+            recursive: true, // required to enter rimraf path
+            maxRetries: 3,
+          });
+        } catch (err) {
+          throw err;
+        }
+      }
+      bench.end(n);
+      break;
+    }
+    case 'non-existing': {
+      bench.start();
+      for (let i = 0; i < n; i++) {
+        try {
+          fs.rmSync(tmpdir.resolve(`.non-existent-folder-${i}`), {
+            recursive: true, // required to enter rimraf path
+            maxRetries: 3,
+          });
+        } catch (err) {
+          assert.ok(err);
+        }
+      }
+      bench.end(n);
+      break;
+    }
+    default:
+      new Error('Invalid type');
+  }
+}

benchmark/fs/bench-rmdirSync.js

@@ -0,0 +1,51 @@
+'use strict';
+
+const assert = require('assert');
+const common = require('../common');
+const fs = require('fs');
+const tmpdir = require('../../test/common/tmpdir');
+tmpdir.refresh();
+
+const bench = common.createBenchmark(main, {
+  type: ['existing', 'non-existing'],
+  n: [1e3],
+});
+
+function main({ n, type }) {
+  switch (type) {
+    case 'existing': {
+      for (let i = 0; i < n; i++) {
+        fs.mkdirSync(tmpdir.resolve(`rmsync-bench-dir-${i}`), {
+          recursive: true,
+        });
+      }
+
+      bench.start();
+      for (let i = 0; i < n; i++) {
+        fs.rmSync(tmpdir.resolve(`rmsync-bench-dir-${i}`), {
+          recursive: true,
+          maxRetries: 3,
+        });
+      }
+      bench.end(n);
+      break;
+    }
+    case 'non-existing': {
+      bench.start();
+      for (let i = 0; i < n; i++) {
+        try {
+          fs.rmSync(tmpdir.resolve(`.non-existent-folder-${i}`), {
+            recursive: true,
+            maxRetries: 3,
+          });
+        } catch (err) {
+          assert.match(err.message, /ENOENT/);
+        }
+      }
+      bench.end(n);
+      break;
+    }
+    default:
+      new Error('Invalid type');
+  }
+}

lib/internal/fs/rimraf.js

@@ -14,6 +14,7 @@ const {
 
 const { Buffer } = require('buffer');
 const fs = require('fs');
+const fsBinding = internalBinding('fs');
 const {
   chmod,
   chmodSync,
@@ -26,7 +27,6 @@ const {
   stat,
   statSync,
   unlink,
-  unlinkSync,
 } = fs;
 const { sep } = require('path');
 const { setTimeout } = require('timers');
@@ -40,7 +40,6 @@ const epermHandlerSync = isWindows ? fixWinEPERMSync : _rmdirSync;
 const readdirEncoding = 'buffer';
 const separator = Buffer.from(sep);
 
-
 function rimraf(path, options, callback) {
   let retries = 0;
 
@@ -192,7 +191,7 @@ function rimrafSync(path, options) {
     if (stats?.isDirectory())
       _rmdirSync(path, options, null);
     else
-      _unlinkSync(path, options);
+      fsBinding.rimrafUnlinkSync(path, options.maxRetries, options.retryDelay);
   } catch (err) {
     if (err.code === 'ENOENT')
       return;
@@ -205,31 +204,6 @@ function rimrafSync(path, options) {
   }
 }
 
-
-function _unlinkSync(path, options) {
-  const tries = options.maxRetries + 1;
-
-  for (let i = 1; i <= tries; i++) {
-    try {
-      return unlinkSync(path);
-    } catch (err) {
-      // Only sleep if this is not the last try, and the delay is greater
-      // than zero, and an error was encountered that warrants a retry.
-      if (retryErrorCodes.has(err.code) &&
-          i < tries &&
-          options.retryDelay > 0) {
-        sleep(i * options.retryDelay);
-      } else if (err.code === 'ENOENT') {
-        // The file is already gone.
-        return;
-      } else if (i === tries) {
-        throw err;
-      }
-    }
-  }
-}
-
-
 function _rmdirSync(path, options, originalErr) {
   try {
     rmdirSync(path);
@@ -303,7 +277,7 @@ function fixWinEPERMSync(path, options, originalErr) {
   if (stats.isDirectory())
     _rmdirSync(path, options, originalErr);
   else
-    _unlinkSync(path, options);
+    fsBinding.rimrafUnlinkSync(path, options.maxRetries, options.retryDelay);
 }
 
 

node.gyp

@@ -106,6 +106,7 @@
       'src/node_errors.cc',
       'src/node_external_reference.cc',
       'src/node_file.cc',
+      'src/node_file_rimraf.cc',
       'src/node_http_parser.cc',
       'src/node_http2.cc',
       'src/node_i18n.cc',

src/node_file.h

@@ -3,7 +3,7 @@
 
 #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
 
-#include <optional>
+//#include <optional>
 #include "aliased_buffer.h"
 #include "node_messaging.h"
 #include "node_snapshotable.h"

src/node_file_rimraf.cc

@@ -0,0 +1,111 @@
+#include "node_file.h"  // NOLINT(build/include_inline)
+#include "node_file-inl.h"
+
+#include <optional>
+
+#if defined(_WIN32)
+#include<windows.h>           // for windows
+#else
+#include<unistd.h>               // for linux
+#endif
+
+namespace node {
+
+namespace fs {
+
+using v8::FunctionCallbackInfo;
+using v8::Int32;
+using v8::Isolate;
+using v8::Value;
+
+#define TRACE_NAME(name) "fs.sync." #name
+#define GET_TRACE_ENABLED                                                      \
+  (*TRACE_EVENT_API_GET_CATEGORY_GROUP_ENABLED(                                \
+       TRACING_CATEGORY_NODE2(fs, sync)) != 0)
+#define FS_SYNC_TRACE_BEGIN(syscall, ...)                                      \
+  if (GET_TRACE_ENABLED)                                                       \
+    TRACE_EVENT_BEGIN(                                                         \
+        TRACING_CATEGORY_NODE2(fs, sync), TRACE_NAME(syscall), ##__VA_ARGS__);
+#define FS_SYNC_TRACE_END(syscall, ...)                                        \
+  if (GET_TRACE_ENABLED)                                                       \
+    TRACE_EVENT_END(                                                           \
+        TRACING_CATEGORY_NODE2(fs, sync), TRACE_NAME(syscall), ##__VA_ARGS__);
+
+static void UnlinkSync(char* path, uint32_t max_retries, uint32_t retry_delay) {
+  Isolate* isolate = Isolate::GetCurrent();
+  Environment* env = Environment::GetCurrent(isolate);
+
+  THROW_IF_INSUFFICIENT_PERMISSIONS(
+      env, permission::PermissionScope::kFileSystemWrite, path);
+
+  const auto tries = max_retries + 1;
+  constexpr std::array<int, 5> retryable_errors = {
+      EBUSY, EMFILE, ENFILE, ENOTEMPTY, EPERM};
+
+  uv_fs_t req;
+
+  for (uint64_t i = 1; i <= tries; i++) {
+    FS_SYNC_TRACE_BEGIN(unlink);
+    auto err = uv_fs_unlink(nullptr, &req, path, nullptr);
+    FS_SYNC_TRACE_END(unlink);
+
+    if(!is_uv_error(err)) {
+      return;
+    }
+
+    if (i < tries && retry_delay > 0 &&
+        std::find(retryable_errors.begin(), retryable_errors.end(), err) != retryable_errors.end()) {
+      sleep(i * retry_delay * 1e-3);
+    } else if (err == UV_ENOENT) {
+      return;
+    } else if (i == tries) {
+      env->ThrowUVException(err, nullptr, "unlink");
+      return;
+    }
+  }
+}
+
+constexpr bool is_uv_error_enoent(int result) {
+  return result == UV_ENOENT;
+}
+
+static void FixWINEPERMSync(char* path, uint32_t max_retries, uint32_t retry_delay) {
+  int chmod_result;
+  uv_fs_t chmod_req;
+  FS_SYNC_TRACE_BEGIN(chmod);
+  chmod_result = uv_fs_chmod(nullptr, &chmod_req, path, 0666, nullptr);
+  FS_SYNC_TRACE_END(chmod);
+
+  if(is_uv_error(chmod_result)) {
+    if (chmod_result == UV_ENOENT) {
+      return;
+    } else {
+      // @TODO throw original error
+      return;
+    }
+  }
+
+  int stat_result;
+  uv_fs_t stat_req;
+  FS_SYNC_TRACE_BEGIN(stat);
+  stat_result = uv_fs_stat(nullptr, &stat_req, path, nullptr);
+  FS_SYNC_TRACE_END(stat);
+
+  if(is_uv_error(stat_result)) {
+    if(stat_result != UV_ENOENT){
+      // @TODO throw original error
+    }
+    return;
+  }
+
+  if(S_ISDIR(stat_req.statbuf.st_mode)) {
+    // @TODO call rmdirSync
+  } else {
+    return UnlinkSync(path, max_retries, retry_delay);
+  }
+}
+
+
+} // end namespace fs
+
+}  // end namespace node

typings/internalBinding/fs.d.ts

@@ -195,6 +195,8 @@ declare namespace InternalFSBinding {
   function rmdir(path: string, req: undefined, ctx: FSSyncContext): void;
   function rmdir(path: string, usePromises: typeof kUsePromises): Promise<void>;
 
+  function rimrafUnlinkSync(path: StringOrBuffer, maxRetries: number, retryDelay: number): void;
+
   function stat(path: StringOrBuffer, useBigint: boolean, req: FSReqCallback<Float64Array | BigUint64Array>): void;
   function stat(path: StringOrBuffer, useBigint: true, req: FSReqCallback<BigUint64Array>): void;
   function stat(path: StringOrBuffer, useBigint: false, req: FSReqCallback<Float64Array>): void;
@@ -275,6 +277,7 @@ export interface FsBinding {
   realpath: typeof InternalFSBinding.realpath;
   rename: typeof InternalFSBinding.rename;
   rmdir: typeof InternalFSBinding.rmdir;
+  rimrafUnlinkSync: typeof InternalFSBinding.rimrafUnlinkSync;
   stat: typeof InternalFSBinding.stat;
   symlink: typeof InternalFSBinding.symlink;
   unlink: typeof InternalFSBinding.unlink;