Merge branch 'main' into fix-49848

Author: MrJithil

benchmark/fs/readFileSync.js

@@ -6,12 +6,21 @@ const fs = require('fs');
 const bench = common.createBenchmark(main, {
   encoding: ['undefined', 'utf8'],
   path: ['existing', 'non-existing'],
-  n: [60e1],
+  hasFileDescriptor: ['true', 'false'],
+  n: [1e4],
 });
 
-function main({ n, encoding, path }) {
+function main({ n, encoding, path, hasFileDescriptor }) {
   const enc = encoding === 'undefined' ? undefined : encoding;
-  const file = path === 'existing' ? __filename : '/tmp/not-found';
+  let file;
+  let shouldClose = false;
+
+  if (hasFileDescriptor === 'true') {
+    shouldClose = path === 'existing';
+    file = path === 'existing' ? fs.openSync(__filename) : -1;
+  } else {
+    file = path === 'existing' ? __filename : '/tmp/not-found';
+  }
   bench.start();
   for (let i = 0; i < n; ++i) {
     try {
@@ -21,4 +30,7 @@ function main({ n, encoding, path }) {
     }
   }
   bench.end(n);
+  if (shouldClose) {
+    fs.closeSync(file);
+  }
 }

common.gypi

@@ -36,7 +36,7 @@
 
     # Reset this number to 0 on major V8 upgrades.
     # Increment by one for each non-official patch applied to deps/v8.
-    'v8_embedder_string': '-node.17',
+    'v8_embedder_string': '-node.18',
 
     ##### V8 defaults for Node.js #####
 

deps/v8/src/flags/flags.cc

@@ -10,6 +10,7 @@
 #include <cstdlib>
 #include <cstring>
 #include <iomanip>
+#include <set>
 #include <sstream>
 
 #include "src/base/functional.h"
@@ -103,7 +104,12 @@ struct Flag {
   const char* cmt_;     // A comment about the flags purpose.
   bool owns_ptr_;       // Does the flag own its string value?
   SetBy set_by_ = SetBy::kDefault;
+  // Name of the flag implying this flag, if any.
   const char* implied_by_ = nullptr;
+#ifdef DEBUG
+  // Pointer to the flag implying this flag, if any.
+  const Flag* implied_by_ptr_ = nullptr;
+#endif
 
   FlagType type() const { return type_; }
 
@@ -113,6 +119,17 @@ struct Flag {
 
   bool PointsTo(const void* ptr) const { return valptr_ == ptr; }
 
+#ifdef DEBUG
+  bool ImpliedBy(const void* ptr) const {
+    const Flag* current = this->implied_by_ptr_;
+    while (current != nullptr) {
+      if (current->PointsTo(ptr)) return true;
+      current = current->implied_by_ptr_;
+    }
+    return false;
+  }
+#endif
+
   bool bool_variable() const { return GetValue<TYPE_BOOL, bool>(); }
 
   void set_bool_variable(bool value, SetBy set_by) {
@@ -333,6 +350,15 @@ struct Flag {
     if (IsAnyImplication(new_set_by)) {
       DCHECK_NOT_NULL(implied_by);
       implied_by_ = implied_by;
+#ifdef DEBUG
+      // This only works when implied_by is a flag_name or !flag_name, but it
+      // can also be a condition e.g. flag_name > 3. Since this is only used for
+      // checks in DEBUG mode, we will just ignore the more complex conditions
+      // for now - that will just lead to a nullptr which won't be followed.
+      implied_by_ptr_ = static_cast<Flag*>(
+          FindFlagByName(implied_by[0] == '!' ? implied_by + 1 : implied_by));
+      DCHECK_NE(implied_by_ptr_, this);
+#endif
     }
     return change_flag;
   }
@@ -534,15 +560,70 @@ uint32_t ComputeFlagListHash() {
   std::ostringstream modified_args_as_string;
   if (COMPRESS_POINTERS_BOOL) modified_args_as_string << "ptr-compr";
   if (DEBUG_BOOL) modified_args_as_string << "debug";
+
+#ifdef DEBUG
+  // These two sets are used to check that we don't leave out any flags
+  // implied by --predictable in the list below.
+  std::set<const char*> flags_implied_by_predictable;
+  std::set<const char*> flags_ignored_because_of_predictable;
+#endif
+
   for (const Flag& flag : flags) {
     if (flag.IsDefault()) continue;
+#ifdef DEBUG
+    if (flag.ImpliedBy(&v8_flags.predictable)) {
+      flags_implied_by_predictable.insert(flag.name());
+    }
+#endif
     // We want to be able to flip --profile-deserialization without
     // causing the code cache to get invalidated by this hash.
     if (flag.PointsTo(&v8_flags.profile_deserialization)) continue;
-    // Skip v8_flags.random_seed to allow predictable code caching.
+    // Skip v8_flags.random_seed and v8_flags.predictable to allow predictable
+    // code caching.
     if (flag.PointsTo(&v8_flags.random_seed)) continue;
+    if (flag.PointsTo(&v8_flags.predictable)) continue;
+
+    // The following flags are implied by --predictable (some negated).
+    if (flag.PointsTo(&v8_flags.concurrent_sparkplug) ||
+        flag.PointsTo(&v8_flags.concurrent_recompilation) ||
+#ifdef V8_ENABLE_MAGLEV
+        flag.PointsTo(&v8_flags.maglev_deopt_data_on_background) ||
+        flag.PointsTo(&v8_flags.maglev_build_code_on_background) ||
+#endif
+        flag.PointsTo(&v8_flags.parallel_scavenge) ||
+        flag.PointsTo(&v8_flags.concurrent_marking) ||
+        flag.PointsTo(&v8_flags.concurrent_array_buffer_sweeping) ||
+        flag.PointsTo(&v8_flags.parallel_marking) ||
+        flag.PointsTo(&v8_flags.concurrent_sweeping) ||
+        flag.PointsTo(&v8_flags.parallel_compaction) ||
+        flag.PointsTo(&v8_flags.parallel_pointer_update) ||
+        flag.PointsTo(&v8_flags.memory_reducer) ||
+        flag.PointsTo(&v8_flags.cppheap_concurrent_marking) ||
+        flag.PointsTo(&v8_flags.cppheap_incremental_marking) ||
+        flag.PointsTo(&v8_flags.single_threaded_gc)) {
+#ifdef DEBUG
+      if (flag.ImpliedBy(&v8_flags.predictable)) {
+        flags_ignored_because_of_predictable.insert(flag.name());
+      }
+#endif
+      continue;
+    }
     modified_args_as_string << flag;
   }
+
+#ifdef DEBUG
+  for (const char* name : flags_implied_by_predictable) {
+    if (flags_ignored_because_of_predictable.find(name) ==
+        flags_ignored_because_of_predictable.end()) {
+      PrintF(
+          "%s should be added to the list of "
+          "flags_ignored_because_of_predictable\n",
+          name);
+      UNREACHABLE();
+    }
+  }
+#endif
+
   std::string args(modified_args_as_string.str());
   // Generate a hash that is not 0.
   uint32_t hash = static_cast<uint32_t>(base::hash_range(

doc/api/deprecations.md

@@ -3414,6 +3414,20 @@ Type: Documentation-only
 The [`util.toUSVString()`][] API is deprecated. Please use
 [`String.prototype.toWellFormed`][] instead.
 
+### DEP0176: `fs.F_OK`, `fs.R_OK`, `fs.W_OK`, `fs.X_OK`
+
+<!-- YAML
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/49683
+    description: Documentation-only deprecation.
+-->
+
+Type: Documentation-only
+
+`F_OK`, `R_OK`, `W_OK` and `X_OK` getters exposed directly on `node:fs` are
+deprecated. Get them from `fs.constants` or `fs.promises.constants` instead.
+
 [NIST SP 800-38D]: https://nvlpubs.nist.gov/nistpubs/Legacy/SP/nistspecialpublication800-38d.pdf
 [RFC 6066]: https://tools.ietf.org/html/rfc6066#section-3
 [RFC 8247 Section 2.4]: https://www.rfc-editor.org/rfc/rfc8247#section-2.4

doc/api/fs.md

@@ -1830,6 +1830,10 @@ concurrent modifications on the same file or data corruption may occur.
 <!-- YAML
 added: v0.11.15
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/49683
+    description: The constants `fs.F_OK`, `fs.R_OK`, `fs.W_OK` and `fs.X_OK`
+                 which were present directly on `fs` are deprecated.
   - version: v18.0.0
     pr-url: https://github.com/nodejs/node/pull/41678
     description: Passing an invalid callback to the `callback` argument

lib/fs.js

@@ -437,13 +437,11 @@ function tryReadSync(fd, isUserFd, buffer, pos, len) {
 function readFileSync(path, options) {
   options = getOptions(options, { flag: 'r' });
 
-  const isUserFd = isFd(path); // File descriptor ownership
-
-  // TODO(@anonrig): Do not handle file descriptor ownership for now.
-  if (!isUserFd && (options.encoding === 'utf8' || options.encoding === 'utf-8')) {
+  if (options.encoding === 'utf8' || options.encoding === 'utf-8') {
     return syncFs.readFileUtf8(path, options.flag);
   }
 
+  const isUserFd = isFd(path); // File descriptor ownership
   const fd = isUserFd ? path : fs.openSync(path, options.flag, 0o666);
 
   const stats = tryStatSync(fd, isUserFd);

lib/internal/fs/sync.js

@@ -9,7 +9,7 @@ const {
   getStatFsFromBinding,
   getValidatedFd,
 } = require('internal/fs/utils');
-const { parseFileMode } = require('internal/validators');
+const { parseFileMode, isInt32 } = require('internal/validators');
 
 const binding = internalBinding('fs');
 
@@ -19,7 +19,9 @@ const binding = internalBinding('fs');
  * @return {string}
  */
 function readFileUtf8(path, flag) {
-  path = pathModule.toNamespacedPath(getValidatedPath(path));
+  if (!isInt32(path)) {
+    path = pathModule.toNamespacedPath(getValidatedPath(path));
+  }
   return binding.readFileUtf8(path, stringToFlags(flag));
 }
 

src/node_file.cc

@@ -2155,7 +2155,7 @@ static void OpenSync(const FunctionCallbackInfo<Value>& args) {
   uv_fs_t req;
   auto make = OnScopeLeave([&req]() { uv_fs_req_cleanup(&req); });
   FS_SYNC_TRACE_BEGIN(open);
-  int err = uv_fs_open(nullptr, &req, *path, flags, mode, nullptr);
+  auto err = uv_fs_open(nullptr, &req, *path, flags, mode, nullptr);
   FS_SYNC_TRACE_END(open);
   if (err < 0) {
     return env->ThrowUVException(err, "open", nullptr, path.out());
@@ -2546,30 +2546,41 @@ static void ReadFileUtf8(const FunctionCallbackInfo<Value>& args) {
 
   CHECK_GE(args.Length(), 2);
 
-  BufferValue path(env->isolate(), args[0]);
-  CHECK_NOT_NULL(*path);
-
   CHECK(args[1]->IsInt32());
   const int flags = args[1].As<Int32>()->Value();
 
-  if (CheckOpenPermissions(env, path, flags).IsNothing()) return;
-
+  uv_file file;
   uv_fs_t req;
-  auto defer_req_cleanup = OnScopeLeave([&req]() { uv_fs_req_cleanup(&req); });
 
-  FS_SYNC_TRACE_BEGIN(open);
-  uv_file file = uv_fs_open(nullptr, &req, *path, flags, 438, nullptr);
-  FS_SYNC_TRACE_END(open);
-  if (req.result < 0) {
-    // req will be cleaned up by scope leave.
-    return env->ThrowUVException(req.result, "open", nullptr, path.out());
+  bool is_fd = args[0]->IsInt32();
+
+  // Check for file descriptor
+  if (is_fd) {
+    file = args[0].As<Int32>()->Value();
+  } else {
+    BufferValue path(env->isolate(), args[0]);
+    CHECK_NOT_NULL(*path);
+    if (CheckOpenPermissions(env, path, flags).IsNothing()) return;
+
+    FS_SYNC_TRACE_BEGIN(open);
+    file = uv_fs_open(nullptr, &req, *path, flags, O_RDONLY, nullptr);
+    FS_SYNC_TRACE_END(open);
+    if (req.result < 0) {
+      uv_fs_req_cleanup(&req);
+      // req will be cleaned up by scope leave.
+      return env->ThrowUVException(req.result, "open", nullptr, path.out());
+    }
   }
 
-  auto defer_close = OnScopeLeave([file]() {
-    uv_fs_t close_req;
-    CHECK_EQ(0, uv_fs_close(nullptr, &close_req, file, nullptr));
-    uv_fs_req_cleanup(&close_req);
+  auto defer_close = OnScopeLeave([file, is_fd, &req]() {
+    if (!is_fd) {
+      FS_SYNC_TRACE_BEGIN(close);
+      CHECK_EQ(0, uv_fs_close(nullptr, &req, file, nullptr));
+      FS_SYNC_TRACE_END(close);
+    }
+    uv_fs_req_cleanup(&req);
   });
+
   std::string result{};
   char buffer[8192];
   uv_buf_t buf = uv_buf_init(buffer, sizeof(buffer));
@@ -2580,7 +2591,7 @@ static void ReadFileUtf8(const FunctionCallbackInfo<Value>& args) {
     if (req.result < 0) {
       FS_SYNC_TRACE_END(read);
       // req will be cleaned up by scope leave.
-      return env->ThrowUVException(req.result, "read", nullptr, path.out());
+      return env->ThrowUVException(req.result, "read", nullptr);
     }
     if (r <= 0) {
       break;

test/parallel/parallel.status

@@ -5,6 +5,8 @@ prefix parallel
 # sample-test                        : PASS,FLAKY
 
 [true] # This section applies to all platforms
+# https://github.com/nodejs/node/issues/49853
+test-runner-output: PASS,FLAKY
 
 [$system==win32]
 # https://github.com/nodejs/node/issues/41206

test/parallel/test-trace-events-bootstrap.js

@@ -32,7 +32,7 @@ if (process.argv[2] === 'child') {
     const file = tmpdir.resolve('node_trace.1.log');
 
     assert(fs.existsSync(file));
-    fs.readFile(file, common.mustCall((err, data) => {
+    fs.readFile(file, common.mustSucceed((data) => {
       const traces = JSON.parse(data.toString()).traceEvents
         .filter((trace) => trace.cat !== '__metadata');
       traces.forEach((trace) => {