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: #34303 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>

Author: addaleax

src/env-inl.h

@@ -871,6 +871,10 @@ inline bool Environment::owns_inspector() const{
 return flags_ & EnvironmentFlags::kOwnsInspector;
 }
 
+inlineboolEnvironment::tracks_unmanaged_fds() const{
+return flags_ & EnvironmentFlags::kTrackUnmanagedFds;
+}
+
 inlineuint64_tEnvironment::thread_id() const{
 return thread_id_;
 }

src/env.cc

@@ -680,6 +680,12 @@ void Environment::RunCleanup(){
  }
 CleanupHandles();
  }
+
+for (constint fd : unmanaged_fds_){
+uv_fs_t close_req;
+uv_fs_close(nullptr, &close_req, fd, nullptr);
+uv_fs_req_cleanup(&close_req);
+ }
 }
 
 voidEnvironment::RunAtExitCallbacks(){
@@ -1045,6 +1051,24 @@ Environment* Environment::worker_parent_env() const{
 returnworker_context()->env();
 }
 
+voidEnvironment::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);
+ }
+}
+
+voidEnvironment::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);
+ }
+}
+
 voidEnvironment::BuildEmbedderGraph(Isolate* isolate,
  EmbedderGraph* graph,
 void* data){

src/env.h

@@ -1066,6 +1066,7 @@ class Environment : public MemoryRetainer{
 inlineboolshould_not_register_esm_loader() const;
 inlineboolowns_process_state() const;
 inlineboolowns_inspector() const;
+inlinebooltracks_unmanaged_fds() const;
 inlineuint64_tthread_id() const;
 inline worker::Worker* worker_context() const;
  Environment* worker_parent_env() const;
@@ -1277,6 +1278,9 @@ class Environment : public MemoryRetainer{
 voidRunAndClearNativeImmediates(bool only_refed = false);
 voidRunAndClearInterrupts();
 
+voidAddUnmanagedFd(int fd);
+voidRemoveUnmanagedFd(int fd);
+
 private:
 inlinevoidThrowError(v8::Local<v8::Value> (*fun)(v8::Local<v8::String>),
 const char* errmsg);
@@ -1432,6 +1436,8 @@ class Environment : public MemoryRetainer{
  ArrayBufferAllocatorList;
  ArrayBufferAllocatorList* keep_alive_allocators_ = nullptr;
 
+ std::unordered_set<int> unmanaged_fds_;
+
  std::function<void(Environment*, int)> process_exit_handler_{
  DefaultProcessExitHandler };
 

src/node.h

@@ -400,7 +400,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
 

src/node_file.cc

@@ -604,6 +604,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));
 }
@@ -813,6 +816,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(env, args[1]);
 if (req_wrap_async != nullptr){// close(fd, req)
@@ -1653,6 +1657,7 @@ static void Open(const FunctionCallbackInfo<Value>& args){
 
  FSReqBase* req_wrap_async = GetReqWrap(env, 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)
@@ -1662,6 +1667,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);
  }
 }

src/node_file.h

@@ -66,6 +66,9 @@ class FSReqBase : public ReqWrap<uv_fs_t>{
 constchar* data() const{return has_data_ ? *buffer_ : nullptr}
 enum encoding encoding() const{return encoding_}
 booluse_bigint() const{return use_bigint_}
+boolis_plain_open() const{return is_plain_open_}
+
+voidset_is_plain_open(bool value){is_plain_open_ = value}
 
  FSContinuationData* continuation_data() const{
 return continuation_data_.get();
@@ -87,8 +90,9 @@ class FSReqBase : public ReqWrap<uv_fs_t>{
  std::unique_ptr<FSContinuationData> continuation_data_;
 enum encoding encoding_ = UTF8;
 bool has_data_ = false;
-constchar* syscall_ = nullptr;
 bool use_bigint_ = false;
+bool is_plain_open_ = false;
+constchar* syscall_ = nullptr;
 
 // Typically, the content of buffer_ is something like a file name, so
 // something around 64 bytes should be enough.