Skip to content

Commit

Permalink
src: cleanup per env handles directly without a list
Browse files Browse the repository at this point in the history
Environment handles can be cleaned up directly without saving the
references in a list and iterate the list.

PR-URL: #54993
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
  • Loading branch information
legendecas authored and marco-ippolito committed Nov 17, 2024
1 parent 6f53c09 commit 7d01b6a
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 46 deletions.
6 changes: 0 additions & 6 deletions src/env-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -260,12 +260,6 @@ inline uv_idle_t* Environment::immediate_idle_handle() {
return &immediate_idle_handle_;
}

inline void Environment::RegisterHandleCleanup(uv_handle_t* handle,
HandleCleanupCb cb,
void* arg) {
handle_cleanup_queue_.push_back(HandleCleanup{handle, cb, arg});
}

template <typename T, typename OnCloseCallback>
inline void Environment::CloseHandle(T* handle, OnCloseCallback callback) {
handle_cleanup_waiting_++;
Expand Down
40 changes: 16 additions & 24 deletions src/env.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1109,13 +1109,8 @@ void Environment::InitializeLibuv() {
}
}

// Register clean-up cb to be called to clean up the handles
// when the environment is freed, note that they are not cleaned in
// the one environment per process setup, but will be called in
// FreeEnvironment.
RegisterHandleCleanups();

StartProfilerIdleNotifier();
env_handle_initialized_ = true;
}

void Environment::ExitEnv(StopFlags::Flags flags) {
Expand All @@ -1136,27 +1131,27 @@ void Environment::ExitEnv(StopFlags::Flags flags) {
});
}

void Environment::RegisterHandleCleanups() {
HandleCleanupCb close_and_finish = [](Environment* env, uv_handle_t* handle,
void* arg) {
handle->data = env;
void Environment::ClosePerEnvHandles() {
// If LoadEnvironment and InitializeLibuv are not called, like when building
// snapshots, skip closing the per environment handles.
if (!env_handle_initialized_) {
return;
}

env->CloseHandle(handle, [](uv_handle_t* handle) {
auto close_and_finish = [&](uv_handle_t* handle) {
CloseHandle(handle, [](uv_handle_t* handle) {
#ifdef DEBUG
memset(handle, 0xab, uv_handle_size(handle->type));
#endif
});
};

auto register_handle = [&](uv_handle_t* handle) {
RegisterHandleCleanup(handle, close_and_finish, nullptr);
};
register_handle(reinterpret_cast<uv_handle_t*>(timer_handle()));
register_handle(reinterpret_cast<uv_handle_t*>(immediate_check_handle()));
register_handle(reinterpret_cast<uv_handle_t*>(immediate_idle_handle()));
register_handle(reinterpret_cast<uv_handle_t*>(&idle_prepare_handle_));
register_handle(reinterpret_cast<uv_handle_t*>(&idle_check_handle_));
register_handle(reinterpret_cast<uv_handle_t*>(&task_queues_async_));
close_and_finish(reinterpret_cast<uv_handle_t*>(timer_handle()));
close_and_finish(reinterpret_cast<uv_handle_t*>(immediate_check_handle()));
close_and_finish(reinterpret_cast<uv_handle_t*>(immediate_idle_handle()));
close_and_finish(reinterpret_cast<uv_handle_t*>(&idle_prepare_handle_));
close_and_finish(reinterpret_cast<uv_handle_t*>(&idle_check_handle_));
close_and_finish(reinterpret_cast<uv_handle_t*>(&task_queues_async_));
}

void Environment::CleanupHandles() {
Expand All @@ -1176,10 +1171,6 @@ void Environment::CleanupHandles() {
for (HandleWrap* handle : handle_wrap_queue_)
handle->Close();

for (HandleCleanup& hc : handle_cleanup_queue_)
hc.cb_(this, hc.handle_, hc.arg_);
handle_cleanup_queue_.clear();

while (handle_cleanup_waiting_ != 0 ||
request_waiting_ != 0 ||
!handle_wrap_queue_.IsEmpty()) {
Expand Down Expand Up @@ -1233,6 +1224,7 @@ MaybeLocal<Value> Environment::RunSnapshotDeserializeMain() const {
void Environment::RunCleanup() {
started_cleanup_ = true;
TRACE_EVENT0(TRACING_CATEGORY_NODE1(environment), "RunCleanup");
ClosePerEnvHandles();
// Only BaseObject's cleanups are registered as per-realm cleanup hooks now.
// Defer the BaseObject cleanup after handles are cleaned up.
CleanupHandles();
Expand Down
19 changes: 3 additions & 16 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -683,24 +683,10 @@ class Environment : public MemoryRetainer {
inline const std::vector<std::string>& argv();
const std::string& exec_path() const;

typedef void (*HandleCleanupCb)(Environment* env,
uv_handle_t* handle,
void* arg);
struct HandleCleanup {
uv_handle_t* handle_;
HandleCleanupCb cb_;
void* arg_;
};

void RegisterHandleCleanups();
void CleanupHandles();
void Exit(ExitCode code);
void ExitEnv(StopFlags::Flags flags);

// Register clean-up cb to be called on environment destruction.
inline void RegisterHandleCleanup(uv_handle_t* handle,
HandleCleanupCb cb,
void* arg);
void ClosePerEnvHandles();

template <typename T, typename OnCloseCallback>
inline void CloseHandle(T* handle, OnCloseCallback callback);
Expand Down Expand Up @@ -1090,6 +1076,8 @@ class Environment : public MemoryRetainer {
std::list<binding::DLib> loaded_addons_;
v8::Isolate* const isolate_;
IsolateData* const isolate_data_;

bool env_handle_initialized_ = false;
uv_timer_t timer_handle_;
uv_check_t immediate_check_handle_;
uv_idle_t immediate_idle_handle_;
Expand Down Expand Up @@ -1201,7 +1189,6 @@ class Environment : public MemoryRetainer {
CleanableQueue cleanable_queue_;
HandleWrapQueue handle_wrap_queue_;
ReqWrapQueue req_wrap_queue_;
std::list<HandleCleanup> handle_cleanup_queue_;
int handle_cleanup_waiting_ = 0;
int request_waiting_ = 0;

Expand Down

0 comments on commit 7d01b6a

Please sign in to comment.