From e2deeedc6e28208ccd041fb3746aa0d73bb9b49a Mon Sep 17 00:00:00 2001 From: Aviv Keller <38299977+RedYetiDev@users.noreply.github.com> Date: Thu, 18 Jul 2024 18:25:20 -0400 Subject: [PATCH] Revert "fs: add v8 fast api to closeSync" This reverts commit ed6f45bef86134533550924baa89fd92d5b24f78. PR-URL: https://github.com/nodejs/node/pull/53904 Refs: https://github.com/yarnpkg/berry/issues/6398 Refs: https://github.com/npm/cli/issues/7657 Reviewed-By: Richard Lau Reviewed-By: Yagiz Nizipli Reviewed-By: Luigi Pinca Reviewed-By: Rich Trott --- src/env.cc | 17 +++-------------- src/env.h | 2 +- src/node_external_reference.h | 4 ---- src/node_file.cc | 33 +++------------------------------ 4 files changed, 7 insertions(+), 49 deletions(-) diff --git a/src/env.cc b/src/env.cc index 46549a46a1bab0..799b36aaebdded 100644 --- 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 index 8c8cafe2122098..b2c54e079b57df 100644 --- a/src/env.h +++ b/src/env.h @@ -1027,7 +1027,7 @@ class Environment : public MemoryRetainer { std::unique_ptr 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 void ForEachRealm(T&& iterator) const; diff --git a/src/node_external_reference.h b/src/node_external_reference.h index 3aad28fa17a08e..b80b8727c23fd1 100644 --- 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, - int32_t input, - v8::FastApiCallbackOptions&); using CFunctionWithUint32 = uint32_t (*)(v8::Local, const uint32_t input); using CFunctionWithDoubleReturnDouble = double (*)(v8::Local, @@ -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 index 81f7e8d2dd5391..c59235b51cca9f 100644 --- 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 FileHandle::TransferData::Deserialize( Environment* env, - Local context, + v8::Local context, std::unique_ptr self) { BindingData* bd = Realm::GetBindingData(context); if (bd == nullptr) return {}; @@ -961,7 +960,7 @@ void Access(const FunctionCallbackInfo& args) { } } -static void Close(const FunctionCallbackInfo& args) { +void Close(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); const int argc = args.Length(); @@ -987,30 +986,6 @@ static void Close(const FunctionCallbackInfo& args) { } } -static void FastClose(Local 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& 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);