fs: add v8 fast api to closeSync

PR-URL: nodejs#53627 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Vinícius Lourenço Claro Cardoso <[email protected]> Reviewed-By: Stephen Belanger <[email protected]> Reviewed-By: James M Snell <[email protected]>
dnalborczyk · Jul 15, 2024 · ed6f45b · ed6f45b
1 parent 4174b73
commit ed6f45b
Show file tree

Hide file tree

Showing 4 changed files with 49 additions and 7 deletions.
diff --git a/src/env.cc b/src/env.cc
@@ -1841,12 +1841,23 @@ void Environment::AddUnmanagedFd(int fd) {
   }
 }
 
-void Environment::RemoveUnmanagedFd(int fd) {
+void Environment::RemoveUnmanagedFd(int fd, bool schedule_native_immediate) {
   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);
+    if (schedule_native_immediate) {
+      SetImmediateThreadsafe([&](Environment* env) {
+        ProcessEmitWarning(this,
+                           "File descriptor %d closed but not opened in "
+                           "unmanaged mode",
+                           fd);
+      });
+    } else {
+      ProcessEmitWarning(
+          this,
+          "File descriptor %d closed but not opened in unmanaged mode",
+          fd);
+    }
   }
 }
 

diff --git a/src/env.h b/src/env.h
@@ -1027,7 +1027,7 @@ class Environment : public MemoryRetainer {
   std::unique_ptr<v8::BackingStore> release_managed_buffer(const uv_buf_t& buf);
 
   void AddUnmanagedFd(int fd);
-  void RemoveUnmanagedFd(int fd);
+  void RemoveUnmanagedFd(int fd, bool schedule_native_immediate = false);
 
   template <typename T>
   void ForEachRealm(T&& iterator) const;

diff --git a/src/node_external_reference.h b/src/node_external_reference.h
@@ -47,6 +47,9 @@ using CFunctionCallbackWithUint8ArrayUint32Int64Bool =
                 uint32_t,
                 int64_t,
                 bool);
+using CFunctionWithObjectInt32Fallback = void (*)(v8::Local<v8::Object>,
+                                                  int32_t input,
+                                                  v8::FastApiCallbackOptions&);
 using CFunctionWithUint32 = uint32_t (*)(v8::Local<v8::Value>,
                                          const uint32_t input);
 using CFunctionWithDoubleReturnDouble = double (*)(v8::Local<v8::Value>,
@@ -75,6 +78,7 @@ class ExternalReferenceRegistry {
   V(CFunctionCallbackWithTwoUint8Arrays)                                       \
   V(CFunctionCallbackWithTwoUint8ArraysFallback)                               \
   V(CFunctionCallbackWithUint8ArrayUint32Int64Bool)                            \
+  V(CFunctionWithObjectInt32Fallback)                                          \
   V(CFunctionWithUint32)                                                       \
   V(CFunctionWithDoubleReturnDouble)                                           \
   V(CFunctionWithInt64Fallback)                                                \

diff --git a/src/node_file.cc b/src/node_file.cc
@@ -54,6 +54,7 @@ namespace fs {
 
 using v8::Array;
 using v8::BigInt;
+using v8::CFunction;
 using v8::Context;
 using v8::EscapableHandleScope;
 using v8::FastApiCallbackOptions;
@@ -298,7 +299,7 @@ FileHandle::TransferData::~TransferData() {
 
 BaseObjectPtr<BaseObject> FileHandle::TransferData::Deserialize(
     Environment* env,
-    v8::Local<v8::Context> context,
+    Local<v8::Context> context,
     std::unique_ptr<worker::TransferData> self) {
   BindingData* bd = Realm::GetBindingData<BindingData>(context);
   if (bd == nullptr) return {};
@@ -960,7 +961,7 @@ void Access(const FunctionCallbackInfo<Value>& args) {
   }
 }
 
-void Close(const FunctionCallbackInfo<Value>& args) {
+static void Close(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
 
   const int argc = args.Length();
@@ -986,6 +987,30 @@ void Close(const FunctionCallbackInfo<Value>& args) {
   }
 }
 
+static void FastClose(Local<Object> recv,
+                      int32_t fd,
+                      // NOLINTNEXTLINE(runtime/references) This is V8 api.
+                      v8::FastApiCallbackOptions& options) {
+  Environment* env = Environment::GetCurrent(recv->GetCreationContextChecked());
+
+  uv_fs_t req;
+  FS_SYNC_TRACE_BEGIN(close);
+  int err = uv_fs_close(nullptr, &req, fd, nullptr) < 0;
+  FS_SYNC_TRACE_END(close);
+  uv_fs_req_cleanup(&req);
+
+  if (err < 0) {
+    options.fallback = true;
+  } else {
+    // Note: Only remove unmanaged fds if the close was successful.
+    // RemoveUnmanagedFd() can call ProcessEmitWarning() which calls back into
+    // JS process.emitWarning() and violates the fast API protocol.
+    env->RemoveUnmanagedFd(fd, true /* schedule native immediate */);
+  }
+}
+
+CFunction fast_close_ = CFunction::Make(FastClose);
+
 static void ExistsSync(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
   Isolate* isolate = env->isolate();
@@ -3305,7 +3330,7 @@ static void CreatePerIsolateProperties(IsolateData* isolate_data,
             "getFormatOfExtensionlessFile",
             GetFormatOfExtensionlessFile);
   SetMethod(isolate, target, "access", Access);
-  SetMethod(isolate, target, "close", Close);
+  SetFastMethod(isolate, target, "close", Close, &fast_close_);
   SetMethod(isolate, target, "existsSync", ExistsSync);
   SetMethod(isolate, target, "open", Open);
   SetMethod(isolate, target, "openFileHandle", OpenFileHandle);
@@ -3430,6 +3455,8 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
 
   registry->Register(GetFormatOfExtensionlessFile);
   registry->Register(Close);
+  registry->Register(FastClose);
+  registry->Register(fast_close_.GetTypeInfo());
   registry->Register(ExistsSync);
   registry->Register(Open);
   registry->Register(OpenFileHandle);