Skip to content

Commit

Permalink
Revert "fs: add v8 fast api to closeSync"
Browse files Browse the repository at this point in the history
This reverts commit ed6f45b.

PR-URL: #53904
Refs: yarnpkg/berry#6398
Refs: npm/cli#7657
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
  • Loading branch information
RedYetiDev authored Jul 18, 2024
1 parent 05b7bf0 commit c0c7e86
Show file tree
Hide file tree
Showing 4 changed files with 7 additions and 49 deletions.
17 changes: 3 additions & 14 deletions src/env.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 0 additions & 4 deletions src/node_external_reference.h
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand Down Expand Up @@ -78,7 +75,6 @@ class ExternalReferenceRegistry {
V(CFunctionCallbackWithTwoUint8Arrays) \
V(CFunctionCallbackWithTwoUint8ArraysFallback) \
V(CFunctionCallbackWithUint8ArrayUint32Int64Bool) \
V(CFunctionWithObjectInt32Fallback) \
V(CFunctionWithUint32) \
V(CFunctionWithDoubleReturnDouble) \
V(CFunctionWithInt64Fallback) \
Expand Down
33 changes: 3 additions & 30 deletions src/node_file.cc
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ namespace fs {

using v8::Array;
using v8::BigInt;
using v8::CFunction;
using v8::Context;
using v8::EscapableHandleScope;
using v8::FastApiCallbackOptions;
Expand Down Expand Up @@ -305,7 +304,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 {};
Expand Down Expand Up @@ -967,7 +966,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();
Expand All @@ -993,30 +992,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();
Expand Down Expand Up @@ -3432,7 +3407,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);
Expand Down Expand Up @@ -3558,8 +3533,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);
Expand Down

0 comments on commit c0c7e86

Please sign in to comment.