fs: improve error performance of chownSync

benchmark/fs/bench-chownSync.js

@@ -0,0 +1,46 @@
+'use strict';
+
+const common = require('../common');
+const fs = require('fs');
+const tmpdir = require('../../test/common/tmpdir');
+tmpdir.refresh();
+
+// Windows does not have `getuid` or `getgid`.
+if (process.platform === 'win32') {
+  return;
+}
+
+const tmpfile = tmpdir.resolve(`.existing-file-${process.pid}`);
+fs.writeFileSync(tmpfile, 'this-is-for-a-benchmark', 'utf8');
+
+const bench = common.createBenchmark(main, {
+  type: ['existing', 'non-existing'],
+  n: [1e4],
+});
+
+function main({ n, type }) {
+  const uid = process.getuid();
+  const gid = process.getgid();
+  let path;
+
+  switch (type) {
+    case 'existing':
+      path = tmpfile;
+      break;
+    case 'non-existing':
+      path = tmpdir.resolve(`.non-existing-file-${Date.now()}`);
+      break;
+    default:
+      new Error('Invalid type');
+  }
+
+  bench.start();
+  for (let i = 0; i < n; i++) {
+    try {
+      fs.chownSync(path, uid, gid);
+    } catch {
+      // do nothing
+    }
+  }
+  bench.end(n);
+}

lib/fs.js

@@ -2063,12 +2063,7 @@ function chown(path, uid, gid, callback) {
  * @returns {void}
  */
 function chownSync(path, uid, gid) {
-  path = getValidatedPath(path);
-  validateInteger(uid, 'uid', -1, kMaxUserId);
-  validateInteger(gid, 'gid', -1, kMaxUserId);
-  const ctx = { path };
-  binding.chown(pathModule.toNamespacedPath(path), uid, gid, undefined, ctx);
-  handleErrorFromBinding(ctx);
+  return syncFs.chown(path, uid, gid);
 }
 
 /**

lib/internal/fs/sync.js

@@ -2,14 +2,17 @@
 
 const pathModule = require('path');
 const {
+  constants: {
+    kMaxUserId,
+  },
   getValidatedPath,
   stringToFlags,
   getValidMode,
   getStatsFromBinding,
   getStatFsFromBinding,
   getValidatedFd,
 } = require('internal/fs/utils');
-const { parseFileMode, isInt32 } = require('internal/validators');
+const { parseFileMode, isInt32, validateInteger } = require('internal/validators');
 
 const binding = internalBinding('fs');
 
@@ -88,6 +91,17 @@ function close(fd) {
   return binding.closeSync(fd);
 }
 
+function chown(path, uid, gid) {
+  path = getValidatedPath(path);
+  validateInteger(uid, 'uid', -1, kMaxUserId);
+  validateInteger(gid, 'gid', -1, kMaxUserId);
+  return binding.chownSync(
+    pathModule.toNamespacedPath(path),
+    uid,
+    gid,
+  );
+}
+
 module.exports = {
   readFileUtf8,
   exists,
@@ -97,4 +111,5 @@ module.exports = {
   statfs,
   open,
   close,
+  chown,
 };

src/node_file.cc

@@ -2751,6 +2751,31 @@ static void Chown(const FunctionCallbackInfo<Value>& args) {
   }
 }
 
+static void ChownSync(const FunctionCallbackInfo<Value>& args) {
+  Environment* env = Environment::GetCurrent(args);
+
+  CHECK_GE(args.Length(), 3);
+
+  BufferValue path(env->isolate(), args[0]);
+  CHECK_NOT_NULL(*path);
+  THROW_IF_INSUFFICIENT_PERMISSIONS(
+      env, permission::PermissionScope::kFileSystemWrite, path.ToStringView());
+
+  CHECK(IsSafeJsInt(args[1]));
+  const uv_uid_t uid = static_cast<uv_uid_t>(args[1].As<Integer>()->Value());
+
+  CHECK(IsSafeJsInt(args[2]));
+  const uv_gid_t gid = static_cast<uv_gid_t>(args[2].As<Integer>()->Value());
+
+  uv_fs_t req;
+  auto cleanup = OnScopeLeave([&req]() { uv_fs_req_cleanup(&req); });
+  FS_SYNC_TRACE_BEGIN(chown);
+  int err = uv_fs_chown(nullptr, &req, *path, uid, gid, nullptr);
+  FS_SYNC_TRACE_END(chown);
+  if (err < 0) {
+    return env->ThrowUVException(err, "chown", nullptr, *path);
+  }
+}
 
 /* fs.fchown(fd, uid, gid);
  * Wrapper for fchown(1) / EIO_FCHOWN
@@ -3401,6 +3426,7 @@ static void CreatePerIsolateProperties(IsolateData* isolate_data,
   SetMethod(isolate, target, "fchmod", FChmod);
 
   SetMethod(isolate, target, "chown", Chown);
+  SetMethod(isolate, target, "chownSync", ChownSync);
   SetMethod(isolate, target, "fchown", FChown);
   SetMethod(isolate, target, "lchown", LChown);
 
@@ -3526,6 +3552,7 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
   registry->Register(FChmod);
 
   registry->Register(Chown);
+  registry->Register(ChownSync);
   registry->Register(FChown);
   registry->Register(LChown);
 

typings/internalBinding/fs.d.ts

@@ -65,6 +65,7 @@ declare namespace InternalFSBinding {
   function chown(path: string, uid: number, gid: number, req: FSReqCallback): void;
   function chown(path: string, uid: number, gid: number, req: undefined, ctx: FSSyncContext): void;
   function chown(path: string, uid: number, gid: number, usePromises: typeof kUsePromises): Promise<void>;
+  function chownSync(path: string, uid: number, gid: number): void;
 
   function close(fd: number, req: FSReqCallback): void;
   function close(fd: number, req: undefined, ctx: FSSyncContext): void;
@@ -236,6 +237,7 @@ export interface FsBinding {
   access: typeof InternalFSBinding.access;
   chmod: typeof InternalFSBinding.chmod;
   chown: typeof InternalFSBinding.chown;
+  chownSync: typeof InternalFSBinding.chownSync;
   close: typeof InternalFSBinding.close;
   copyFile: typeof InternalFSBinding.copyFile;
   fchmod: typeof InternalFSBinding.fchmod;