src: add option to track unmanaged file descriptors

Add the ability to track “raw” file descriptors, i.e. integers returned
by `fs.open()`, and close them on `Environment` shutdown, to match the
behavior for all other resource types (which are also closed on
shutdown).

PR-URL: https://github.com/nodejs/node/pull/34303
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

6 files changed, 48 insertions, 1 deletions

diff --git a/src/env-inl.h b/src/env-inl.h
index a1300ade79..ddae576612 100644
--- a/src/env-inl.h
+++ b/src/env-inl.h
@@ -817,6 +817,10 @@ inline bool Environment::owns_inspector() const {
   return flags_ & EnvironmentFlags::kOwnsInspector;
 }
 
+inline bool Environment::tracks_unmanaged_fds() const {
+  return flags_ & EnvironmentFlags::kTrackUnmanagedFds;
+}
+
 bool Environment::filehandle_close_warning() const {
   return emit_filehandle_warning_;
 }
diff --git a/src/env.cc b/src/env.cc
index 1ab9206ce4..bcaa50bd01 100644
--- a/src/env.cc
+++ b/src/env.cc
@@ -619,6 +619,12 @@ void Environment::RunCleanup() {
     }
     CleanupHandles();
   }
+
+  for (const int fd : unmanaged_fds_) {
+    uv_fs_t close_req;
+    uv_fs_close(nullptr, &close_req, fd, nullptr);
+    uv_fs_req_cleanup(&close_req);
+  }
 }
 
 void Environment::RunAtExitCallbacks() {
@@ -981,6 +987,24 @@ Environment* Environment::worker_parent_env() const {
   return worker_context()->env();
 }
 
+void Environment::AddUnmanagedFd(int fd) {
+  if (!tracks_unmanaged_fds()) return;
+  auto result = unmanaged_fds_.insert(fd);
+  if (!result.second) {
+    ProcessEmitWarning(
+        this, "File descriptor %d opened in unmanaged mode twice", fd);
+  }
+}
+
+void Environment::RemoveUnmanagedFd(int fd) {
+  if (!tracks_unmanaged_fds()) return;
+  size_t removed_count = unmanaged_fds_.erase(fd);
+  if (removed_count == 0) {
+    ProcessEmitWarning(
+        this, "File descriptor %d closed but not opened in unmanaged mode", fd);
+  }
+}
+
 void Environment::BuildEmbedderGraph(Isolate* isolate,
                                      EmbedderGraph* graph,
                                      void* data) {
diff --git a/src/env.h b/src/env.h
index c95554bd68..e256a30e90 100644
--- a/src/env.h
+++ b/src/env.h
@@ -1015,6 +1015,7 @@ class Environment : public MemoryRetainer {
   inline bool should_not_register_esm_loader() const;
   inline bool owns_process_state() const;
   inline bool owns_inspector() const;
+  inline bool tracks_unmanaged_fds() const;
   inline uint64_t thread_id() const;
   inline worker::Worker* worker_context() const;
   Environment* worker_parent_env() const;
@@ -1226,6 +1227,9 @@ class Environment : public MemoryRetainer {
   inline std::unordered_map<char*, std::unique_ptr<v8::BackingStore>>*
       released_allocated_buffers();
 
+  void AddUnmanagedFd(int fd);
+  void RemoveUnmanagedFd(int fd);
+
  private:
   inline void ThrowError(v8::Local<v8::Value> (*fun)(v8::Local<v8::String>),
                          const char* errmsg);
@@ -1363,6 +1367,8 @@ class Environment : public MemoryRetainer {
   int64_t initial_base_object_count_ = 0;
   std::atomic_bool is_stopping_ { false };
 
+  std::unordered_set<int> unmanaged_fds_;
+
   std::function<void(Environment*, int)> process_exit_handler_ {
       DefaultProcessExitHandler };
 
diff --git a/src/node.h b/src/node.h
index 4da7b3df80..25f9f9b873 100644
--- a/src/node.h
+++ b/src/node.h
@@ -417,7 +417,10 @@ enum Flags : uint64_t {
   // Set if Node.js should not run its own esm loader. This is needed by some
   // embedders, because it's possible for the Node.js esm loader to conflict
   // with another one in an embedder environment, e.g. Blink's in Chromium.
-  kNoRegisterESMLoader = 1 << 3
+  kNoRegisterESMLoader = 1 << 3,
+  // Set this flag to make Node.js track "raw" file descriptors, i.e. managed
+  // by fs.open() and fs.close(), and close them during FreeEnvironment().
+  kTrackUnmanagedFds = 1 << 4
 };
 }  // namespace EnvironmentFlags
 
diff --git a/src/node_file.cc b/src/node_file.cc
index f1d9824dfe..3e1cecdadc 100644
--- a/src/node_file.cc
+++ b/src/node_file.cc
@@ -653,6 +653,9 @@ void AfterInteger(uv_fs_t* req) {
   FSReqBase* req_wrap = FSReqBase::from_req(req);
   FSReqAfterScope after(req_wrap, req);
 
+  if (req->result >= 0 && req_wrap->is_plain_open())
+    req_wrap->env()->AddUnmanagedFd(req->result);
+
   if (after.Proceed())
     req_wrap->Resolve(Integer::New(req_wrap->env()->isolate(), req->result));
 }
@@ -862,6 +865,7 @@ void Close(const FunctionCallbackInfo<Value>& args) {
 
   CHECK(args[0]->IsInt32());
   int fd = args[0].As<Int32>()->Value();
+  env->RemoveUnmanagedFd(fd);
 
   FSReqBase* req_wrap_async = GetReqWrap(args, 1);
   if (req_wrap_async != nullptr) {  // close(fd, req)
@@ -1706,6 +1710,7 @@ static void Open(const FunctionCallbackInfo<Value>& args) {
 
   FSReqBase* req_wrap_async = GetReqWrap(args, 3);
   if (req_wrap_async != nullptr) {  // open(path, flags, mode, req)
+    req_wrap_async->set_is_plain_open(true);
     AsyncCall(env, req_wrap_async, args, "open", UTF8, AfterInteger,
               uv_fs_open, *path, flags, mode);
   } else {  // open(path, flags, mode, undefined, ctx)
@@ -1715,6 +1720,7 @@ static void Open(const FunctionCallbackInfo<Value>& args) {
     int result = SyncCall(env, args[4], &req_wrap_sync, "open",
                           uv_fs_open, *path, flags, mode);
     FS_SYNC_TRACE_END(open);
+    if (result >= 0) env->AddUnmanagedFd(result);
     args.GetReturnValue().Set(result);
   }
 }
diff --git a/src/node_file.h b/src/node_file.h
index fd17fc99d5..2b157de5eb 100644
--- a/src/node_file.h
+++ b/src/node_file.h
@@ -89,6 +89,9 @@ class FSReqBase : public ReqWrap<uv_fs_t> {
   const char* data() const { return has_data_ ? *buffer_ : nullptr; }
   enum encoding encoding() const { return encoding_; }
   bool use_bigint() const { return use_bigint_; }
+  bool is_plain_open() const { return is_plain_open_; }
+
+  void set_is_plain_open(bool value) { is_plain_open_ = value; }
 
   FSContinuationData* continuation_data() const {
     return continuation_data_.get();
@@ -113,6 +116,7 @@ class FSReqBase : public ReqWrap<uv_fs_t> {
   enum encoding encoding_ = UTF8;
   bool has_data_ = false;
   bool use_bigint_ = false;
+  bool is_plain_open_ = false;
   const char* syscall_ = nullptr;
 
   BaseObjectPtr<BindingData> binding_data_;