FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

src: add Cleanable class to Environment · nodejs/node@85542b0 · GitHub

/ node Public

Commit 85542b0

Browse files
gabrielschulhof
authored and
targos
committed
src: add Cleanable class to Environment
We store a linked list of `Cleanable` objects on the `node::Environment` and invoke their `Clean()` method during env teardown. This eliminates the need for adding many cleanup hooks. PR-URL: #54880 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
1 parent 124f715 commit 85542b0

3 files changed

Lines changed: 31 additions & 12 deletions

File tree

‎src/env.cc‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1237,6 +1237,11 @@ void Environment::RunCleanup() {
12371237
// Defer the BaseObject cleanup after handles are cleaned up.
12381238
CleanupHandles();
12391239

1240+
while (!cleanable_queue_.IsEmpty()) {
1241+
Cleanable* cleanable = cleanable_queue_.PopFront();
1242+
cleanable->Clean();
1243+
}
1244+
12401245
while (!cleanup_queue_.empty() || principal_realm_->HasCleanupHooks() ||
12411246
native_immediates_.size() > 0 ||
12421247
native_immediates_threadsafe_.size() > 0 ||

‎src/env.h‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -599,6 +599,18 @@ void DefaultProcessExitHandlerInternal(Environment* env, ExitCode exit_code);
599599
v8::Maybe<ExitCode> SpinEventLoopInternal(Environment* env);
600600
v8::Maybe<ExitCode> EmitProcessExitInternal(Environment* env);
601601

602+
class Cleanable {
603+
public:
604+
virtual ~Cleanable() = default;
605+
606+
protected:
607+
ListNode<Cleanable> cleanable_queue_;
608+
609+
private:
610+
virtual void Clean() = 0;
611+
friend class Environment;
612+
};
613+
602614
/**
603615
* Environment is a per-isolate data structure that represents an execution
604616
* environment. Each environment has a principal realm. An environment can
@@ -905,8 +917,12 @@ class Environment : public MemoryRetainer {
905917

906918
typedef ListHead<HandleWrap, &HandleWrap::handle_wrap_queue_> HandleWrapQueue;
907919
typedef ListHead<ReqWrapBase, &ReqWrapBase::req_wrap_queue_> ReqWrapQueue;
920+
typedef ListHead<Cleanable, &Cleanable::cleanable_queue_> CleanableQueue;
908921

909922
inline HandleWrapQueue* handle_wrap_queue() { return &handle_wrap_queue_; }
923+
inline CleanableQueue* cleanable_queue() {
924+
return &cleanable_queue_;
925+
}
910926
inline ReqWrapQueue* req_wrap_queue() { return &req_wrap_queue_; }
911927

912928
// https://w3c.github.io/hr-time/#dfn-time-origin
@@ -1182,6 +1198,7 @@ class Environment : public MemoryRetainer {
11821198
// memory are predictable. For more information please refer to
11831199
// `doc/contributing/node-postmortem-support.md`
11841200
friend int GenDebugSymbols();
1201+
CleanableQueue cleanable_queue_;
11851202
HandleWrapQueue handle_wrap_queue_;
11861203
ReqWrapQueue req_wrap_queue_;
11871204
std::list<HandleCleanup> handle_cleanup_queue_;

‎src/node_buffer.cc‎

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ using v8::Value;
8181

8282
namespace {
8383

84-
class CallbackInfo {
84+
class CallbackInfo : public Cleanable {
8585
public:
8686
static inline Local<ArrayBuffer> CreateTrackedArrayBuffer(
8787
Environment* env,
@@ -94,7 +94,7 @@ class CallbackInfo {
9494
CallbackInfo& operator=(const CallbackInfo&) = delete;
9595

9696
private:
97-
static void CleanupHook(void* data);
97+
void Clean();
9898
inline void OnBackingStoreFree();
9999
inline void CallAndResetCallback();
100100
inline CallbackInfo(Environment* env,
@@ -109,7 +109,6 @@ class CallbackInfo {
109109
Environment* const env_;
110110
};
111111

112-
113112
Local<ArrayBuffer> CallbackInfo::CreateTrackedArrayBuffer(
114113
Environment* env,
115114
char* data,
@@ -149,25 +148,23 @@ CallbackInfo::CallbackInfo(Environment* env,
149148
data_(data),
150149
hint_(hint),
151150
env_(env) {
152-
env->AddCleanupHook(CleanupHook, this);
151+
env->cleanable_queue()->PushFront(this);
153152
env->isolate()->AdjustAmountOfExternalAllocatedMemory(sizeof(*this));
154153
}
155154

156-
void CallbackInfo::CleanupHook(void* data) {
157-
CallbackInfo* self = static_cast<CallbackInfo*>(data);
158-
155+
void CallbackInfo::Clean() {
159156
{
160-
HandleScope handle_scope(self->env_->isolate());
161-
Local<ArrayBuffer> ab = self->persistent_.Get(self->env_->isolate());
157+
HandleScope handle_scope(env_->isolate());
158+
Local<ArrayBuffer> ab = persistent_.Get(env_->isolate());
162159
if (!ab.IsEmpty() && ab->IsDetachable()) {
163160
ab->Detach(Local<Value>()).Check();
164-
self->persistent_.Reset();
161+
persistent_.Reset();
165162
}
166163
}
167164

168165
// Call the callback in this case, but don't delete `this` yet because the
169166
// BackingStore deleter callback will do so later.
170-
self->CallAndResetCallback();
167+
CallAndResetCallback();
171168
}
172169

173170
void CallbackInfo::CallAndResetCallback() {
@@ -179,7 +176,7 @@ void CallbackInfo::CallAndResetCallback() {
179176
}
180177
if (callback != nullptr) {
181178
// Clean up all Environment-related state and run the callback.
182-
env_->RemoveCleanupHook(CleanupHook, this);
179+
cleanable_queue_.Remove();
183180
int64_t change_in_bytes = -static_cast<int64_t>(sizeof(*this));
184181
env_->isolate()->AdjustAmountOfExternalAllocatedMemory(change_in_bytes);
185182

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL