src: fix internalModuleStat v8 fast path

PR-URL: #58054
Backport-PR-URL: #59065
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Robert Nagy <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>

Author: anonrig

lib/fs.js

@@ -1448,7 +1448,7 @@ function handleDirents({ result, currentPath, context }) {
     const dirent = getDirent(currentPath, names[i], types[i]);
     ArrayPrototypePush(context.readdirResults, dirent);
 
-    if (dirent.isDirectory() || binding.internalModuleStat(binding, fullPath) === 1) {
+    if (dirent.isDirectory() || binding.internalModuleStat(fullPath) === 1) {
       ArrayPrototypePush(context.pathsQueue, fullPath);
     }
   }
@@ -1458,7 +1458,7 @@ function handleFilePaths({ result, currentPath, context }) {
   for (let i = 0; i < result.length; i++) {
     const resultPath = pathModule.join(currentPath, result[i]);
     const relativeResultPath = pathModule.relative(context.basePath, resultPath);
-    const stat = binding.internalModuleStat(binding, resultPath);
+    const stat = binding.internalModuleStat(resultPath);
     ArrayPrototypePush(context.readdirResults, relativeResultPath);
 
     if (stat === 1) {

lib/internal/fs/promises.js

@@ -915,7 +915,7 @@ async function readdirRecursive(originalPath, options) {
       const { 0: path, 1: readdir } = ArrayPrototypePop(queue);
       for (const ent of readdir) {
         const direntPath = pathModule.join(path, ent);
-        const stat = binding.internalModuleStat(binding, direntPath);
+        const stat = binding.internalModuleStat(direntPath);
         ArrayPrototypePush(
           result,
           pathModule.relative(originalPath, direntPath),

lib/internal/modules/cjs/loader.js

@@ -255,9 +255,9 @@ function stat(filename) {
     const result = statCache.get(filename);
     if (result !== undefined) { return result; }
   }
-  const result = internalFsBinding.internalModuleStat(internalFsBinding, filename);
+  const result = internalFsBinding.internalModuleStat(filename);
   if (statCache !== null && result >= 0) {
-    // Only set cache when `internalModuleStat(internalFsBinding, filename)` succeeds.
+    // Only set cache when `internalModuleStat(filename)` succeeds.
     statCache.set(filename, result);
   }
   return result;

lib/internal/modules/esm/resolve.js

@@ -248,7 +248,6 @@ function finalizeResolution(resolved, base, preserveSymlinks) {
   }
 
   const stats = internalFsBinding.internalModuleStat(
-    internalFsBinding,
     StringPrototypeEndsWith(internalFsBinding, path, '/') ? StringPrototypeSlice(path, -1) : path,
   );
 

lib/internal/modules/package_json_reader.js

@@ -235,7 +235,6 @@ function getPackageJSONURL(specifier, base) {
   let lastPath;
   do {
     const stat = internalFsBinding.internalModuleStat(
-      internalFsBinding,
       StringPrototypeSlice(packageJSONPath, 0, packageJSONPath.length - 13),
     );
     // Check for !stat.isDirectory()

src/node_external_reference.h

@@ -20,9 +20,8 @@ using CFunctionCallback = void (*)(v8::Local<v8::Value> unused,
 using CFunctionCallbackReturnDouble =
     double (*)(v8::Local<v8::Object> unused, v8::Local<v8::Object> receiver);
 using CFunctionCallbackReturnInt32 =
-    int32_t (*)(v8::Local<v8::Object> unused,
-                v8::Local<v8::Object> receiver,
-                const v8::FastOneByteString& input,
+    int32_t (*)(v8::Local<v8::Value> receiver,
+                v8::Local<v8::Value> input,
                 // NOLINTNEXTLINE(runtime/references) This is V8 api.
                 v8::FastApiCallbackOptions& options);
 using CFunctionCallbackValueReturnDouble =

src/node_file.cc

@@ -23,6 +23,7 @@
 #include "aliased_buffer-inl.h"
 #include "memory_tracker-inl.h"
 #include "node_buffer.h"
+#include "node_debug.h"
 #include "node_errors.h"
 #include "node_external_reference.h"
 #include "node_file-inl.h"
@@ -60,8 +61,6 @@ using v8::BigInt;
 using v8::Context;
 using v8::EscapableHandleScope;
 using v8::FastApiCallbackOptions;
-using v8::FastOneByteString;
-using v8::Function;
 using v8::FunctionCallbackInfo;
 using v8::FunctionTemplate;
 using v8::HandleScope;
@@ -1051,9 +1050,9 @@ static void ExistsSync(const FunctionCallbackInfo<Value>& args) {
 static void InternalModuleStat(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
 
-  CHECK_GE(args.Length(), 2);
-  CHECK(args[1]->IsString());
-  BufferValue path(env->isolate(), args[1]);
+  CHECK_EQ(args.Length(), 1);
+  CHECK(args[0]->IsString());
+  BufferValue path(env->isolate(), args[0]);
   CHECK_NOT_NULL(*path);
   ToNamespacedPath(env, &path);
 
@@ -1069,9 +1068,8 @@ static void InternalModuleStat(const FunctionCallbackInfo<Value>& args) {
 }
 
 static int32_t FastInternalModuleStat(
-    Local<Object> unused,
-    Local<Object> recv,
-    const FastOneByteString& input,
+    Local<Value> recv,
+    Local<Value> input_,
     // NOLINTNEXTLINE(runtime/references) This is V8 api.
     FastApiCallbackOptions& options) {
   // This needs a HandleScope which needs an isolate.
@@ -1081,9 +1079,13 @@ static int32_t FastInternalModuleStat(
     return -1;
   }
 
+  TRACK_V8_FAST_API_CALL("fs.internalModuleStat");
   HandleScope scope(isolate);
 
-  auto path = std::filesystem::path(input.data, input.data + input.length);
+  CHECK(input_->IsString());
+  Utf8Value input(isolate, input_.As<String>());
+
+  auto path = std::filesystem::path(input.ToStringView());
 
   switch (std::filesystem::status(path).type()) {
     case std::filesystem::file_type::directory:

test/parallel/test-permission-fs-internal-module-stat.js

@@ -1,8 +1,9 @@
-// Flags: --expose-internals --permission --allow-fs-read=test/common* --allow-fs-read=tools* --allow-fs-read=test/parallel* --allow-child-process
+// Flags: --expose-internals --permission --allow-fs-read=test/common* --allow-fs-read=tools* --allow-fs-read=test/parallel* --allow-child-process --allow-natives-syntax
 'use strict';
 
 const common = require('../common');
 const { isMainThread } = require('worker_threads');
+const { strictEqual } = require('assert');
 
 if (!isMainThread) {
   common.skip('This test only works on a main thread');
@@ -18,9 +19,20 @@ const fixtures = require('../common/fixtures');
 const blockedFile = fixtures.path('permission', 'deny', 'protected-file.md');
 const internalFsBinding = internalBinding('fs');
 
-// Run this inside a for loop to trigger the fast API
-for (let i = 0; i < 10_000; i++) {
-  // internalModuleStat does not use permission model.
-  // doesNotThrow
-  internalFsBinding.internalModuleStat(internalFsBinding, blockedFile);
+strictEqual(internalFsBinding.internalModuleStat(blockedFile), 0);
+
+// Only javascript methods can be optimized through %OptimizeFunctionOnNextCall
+// This is why we surround the C++ method we want to optimize with a JS function.
+function testFastPaths(file) {
+  return internalFsBinding.internalModuleStat(file);
+}
+
+eval('%PrepareFunctionForOptimization(testFastPaths)');
+testFastPaths(blockedFile);
+eval('%OptimizeFunctionOnNextCall(testFastPaths)');
+strictEqual(testFastPaths(blockedFile), 0);
+
+if (common.isDebug) {
+  const { getV8FastApiCallCount } = internalBinding('debug');
+  strictEqual(getV8FastApiCallCount('fs.internalModuleStat'), 1);
 }