From ed6f45bef86134533550924baa89fd92d5b24f78 Mon Sep 17 00:00:00 2001 From: Yagiz Nizipli Date: Sun, 14 Jul 2024 20:28:45 -0400 Subject: [PATCH] fs: add v8 fast api to closeSync MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/53627 Reviewed-By: Matteo Collina Reviewed-By: Vinícius Lourenço Claro Cardoso Reviewed-By: Stephen Belanger Reviewed-By: James M Snell --- src/env.cc | 17 ++++++++++++++--- src/env.h | 2 +- src/node_external_reference.h | 4 ++++ src/node_file.cc | 33 ++++++++++++++++++++++++++++++--- 4 files changed, 49 insertions(+), 7 deletions(-) diff --git a/src/env.cc b/src/env.cc index 799b36aaebdded..46549a46a1bab0 100644 --- 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 index b2c54e079b57df..8c8cafe2122098 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); + void RemoveUnmanagedFd(int fd, bool schedule_native_immediate = false); template void ForEachRealm(T&& iterator) const; diff --git a/src/node_external_reference.h b/src/node_external_reference.h index b80b8727c23fd1..3aad28fa17a08e 100644 --- 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, + int32_t input, + v8::FastApiCallbackOptions&); using CFunctionWithUint32 = uint32_t (*)(v8::Local, const uint32_t input); using CFunctionWithDoubleReturnDouble = double (*)(v8::Local, @@ -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 index c59235b51cca9f..81f7e8d2dd5391 100644 --- 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 FileHandle::TransferData::Deserialize( Environment* env, - v8::Local context, + Local context, std::unique_ptr self) { BindingData* bd = Realm::GetBindingData(context); if (bd == nullptr) return {}; @@ -960,7 +961,7 @@ void Access(const FunctionCallbackInfo& args) { } } -void Close(const FunctionCallbackInfo& args) { +static void Close(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); const int argc = args.Length(); @@ -986,6 +987,30 @@ 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(); @@ -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);