Revert "fs: add v8 fast api to closeSync"

This reverts commit ed6f45b. PR-URL: #53904 Refs: yarnpkg/berry#6398 Refs: npm/cli#7657 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Rich Trott <[email protected]>
nodejs · Jul 19, 2024 · e2deeed · e2deeed
1 parent 6bcaaa6
commit e2deeed
Show file tree

Hide file tree

Showing 4 changed files with 7 additions and 49 deletions.
diff --git a/src/env.cc b/src/env.cc
@@ -1841,23 +1841,12 @@ void Environment::AddUnmanagedFd(int fd) {
   }
 }
 
-void Environment::RemoveUnmanagedFd(int fd, bool schedule_native_immediate) {
+void Environment::RemoveUnmanagedFd(int fd) {
   if (!tracks_unmanaged_fds()) return;
   size_t removed_count = unmanaged_fds_.erase(fd);
   if (removed_count == 0) {
-    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);
-    }
+    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, bool schedule_native_immediate = false);
+  void RemoveUnmanagedFd(int fd);
 
   template <typename T>
   void ForEachRealm(T&& iterator) const;

diff --git a/src/node_external_reference.h b/src/node_external_reference.h
@@ -47,9 +47,6 @@ 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>,
@@ -78,7 +75,6 @@ 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,7 +54,6 @@ namespace fs {
 
 using v8::Array;
 using v8::BigInt;
-using v8::CFunction;
 using v8::Context;
 using v8::EscapableHandleScope;
 using v8::FastApiCallbackOptions;
@@ -299,7 +298,7 @@ FileHandle::TransferData::~TransferData() {
 
 BaseObjectPtr<BaseObject> FileHandle::TransferData::Deserialize(
     Environment* env,
-    Local<v8::Context> context,
+    v8::Local<v8::Context> context,
     std::unique_ptr<worker::TransferData> self) {
   BindingData* bd = Realm::GetBindingData<BindingData>(context);
   if (bd == nullptr) return {};
@@ -961,7 +960,7 @@ void Access(const FunctionCallbackInfo<Value>& args) {
   }
 }
 
-static void Close(const FunctionCallbackInfo<Value>& args) {
+void Close(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
 
   const int argc = args.Length();
@@ -987,30 +986,6 @@ static 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();
@@ -3330,7 +3305,7 @@ static void CreatePerIsolateProperties(IsolateData* isolate_data,
             "getFormatOfExtensionlessFile",
             GetFormatOfExtensionlessFile);
   SetMethod(isolate, target, "access", Access);
-  SetFastMethod(isolate, target, "close", Close, &fast_close_);
+  SetMethod(isolate, target, "close", Close);
   SetMethod(isolate, target, "existsSync", ExistsSync);
   SetMethod(isolate, target, "open", Open);
   SetMethod(isolate, target, "openFileHandle", OpenFileHandle);
@@ -3455,8 +3430,6 @@ 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);