Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit e8101af

Browse filesBrowse files
legendecasRafaelGSS
legendecas
authored and
RafaelGSS
committed
src: consolidate environment cleanup queue
Each Realm tracks its own cleanup hooks and drains the hooks when it is going to be destroyed. Moves the implementations of the cleanup queue to its own class so that it can be used in `node::Realm` too. PR-URL: #44379 Refs: #44348 Refs: #42528 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
1 parent 143abcb commit e8101af
Copy full SHA for e8101af

File tree

Expand file treeCollapse file tree

10 files changed

+219
-106
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

10 files changed

+219
-106
lines changed
Open diff view settings
Collapse file

‎node.gyp‎

Copy file name to clipboardExpand all lines: node.gyp
+3Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -478,6 +478,7 @@
478478
'src/api/utils.cc',
479479
'src/async_wrap.cc',
480480
'src/cares_wrap.cc',
481+
'src/cleanup_queue.cc',
481482
'src/connect_wrap.cc',
482483
'src/connection_wrap.cc',
483484
'src/debug_utils.cc',
@@ -577,6 +578,8 @@
577578
'src/base64-inl.h',
578579
'src/callback_queue.h',
579580
'src/callback_queue-inl.h',
581+
'src/cleanup_queue.h',
582+
'src/cleanup_queue-inl.h',
580583
'src/connect_wrap.h',
581584
'src/connection_wrap.h',
582585
'src/debug_utils.h',
Collapse file

‎src/base_object.h‎

Copy file name to clipboardExpand all lines: src/base_object.h
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ class BaseObject : public MemoryRetainer {
173173
// position of members in memory are predictable. For more information please
174174
// refer to `doc/contributing/node-postmortem-support.md`
175175
friend int GenDebugSymbols();
176-
friend class CleanupHookCallback;
176+
friend class CleanupQueue;
177177
template <typename T, bool kIsWeak>
178178
friend class BaseObjectPtrImpl;
179179

Collapse file

‎src/cleanup_queue-inl.h‎

Copy file name to clipboard
+60Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
#ifndef SRC_CLEANUP_QUEUE_INL_H_
2+
#define SRC_CLEANUP_QUEUE_INL_H_
3+
4+
#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
5+
6+
#include "base_object.h"
7+
#include "cleanup_queue.h"
8+
#include "memory_tracker-inl.h"
9+
#include "util.h"
10+
11+
namespace node {
12+
13+
inline void CleanupQueue::MemoryInfo(MemoryTracker* tracker) const {
14+
ForEachBaseObject([&](BaseObject* obj) {
15+
if (obj->IsDoneInitializing()) tracker->Track(obj);
16+
});
17+
}
18+
19+
inline size_t CleanupQueue::SelfSize() const {
20+
return sizeof(CleanupQueue) +
21+
cleanup_hooks_.size() * sizeof(CleanupHookCallback);
22+
}
23+
24+
bool CleanupQueue::empty() const {
25+
return cleanup_hooks_.empty();
26+
}
27+
28+
void CleanupQueue::Add(Callback cb, void* arg) {
29+
auto insertion_info = cleanup_hooks_.emplace(
30+
CleanupHookCallback{cb, arg, cleanup_hook_counter_++});
31+
// Make sure there was no existing element with these values.
32+
CHECK_EQ(insertion_info.second, true);
33+
}
34+
35+
void CleanupQueue::Remove(Callback cb, void* arg) {
36+
CleanupHookCallback search{cb, arg, 0};
37+
cleanup_hooks_.erase(search);
38+
}
39+
40+
template <typename T>
41+
void CleanupQueue::ForEachBaseObject(T&& iterator) const {
42+
for (const auto& hook : cleanup_hooks_) {
43+
BaseObject* obj = GetBaseObject(hook);
44+
if (obj != nullptr) iterator(obj);
45+
}
46+
}
47+
48+
BaseObject* CleanupQueue::GetBaseObject(
49+
const CleanupHookCallback& callback) const {
50+
if (callback.fn_ == BaseObject::DeleteMe)
51+
return static_cast<BaseObject*>(callback.arg_);
52+
else
53+
return nullptr;
54+
}
55+
56+
} // namespace node
57+
58+
#endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
59+
60+
#endif // SRC_CLEANUP_QUEUE_INL_H_
Collapse file

‎src/cleanup_queue.cc‎

Copy file name to clipboard
+44Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
#include "cleanup_queue.h" // NOLINT(build/include_inline)
2+
#include <vector>
3+
#include "cleanup_queue-inl.h"
4+
5+
namespace node {
6+
7+
void CleanupQueue::Drain() {
8+
// Copy into a vector, since we can't sort an unordered_set in-place.
9+
std::vector<CleanupHookCallback> callbacks(cleanup_hooks_.begin(),
10+
cleanup_hooks_.end());
11+
// We can't erase the copied elements from `cleanup_hooks_` yet, because we
12+
// need to be able to check whether they were un-scheduled by another hook.
13+
14+
std::sort(callbacks.begin(),
15+
callbacks.end(),
16+
[](const CleanupHookCallback& a, const CleanupHookCallback& b) {
17+
// Sort in descending order so that the most recently inserted
18+
// callbacks are run first.
19+
return a.insertion_order_counter_ > b.insertion_order_counter_;
20+
});
21+
22+
for (const CleanupHookCallback& cb : callbacks) {
23+
if (cleanup_hooks_.count(cb) == 0) {
24+
// This hook was removed from the `cleanup_hooks_` set during another
25+
// hook that was run earlier. Nothing to do here.
26+
continue;
27+
}
28+
29+
cb.fn_(cb.arg_);
30+
cleanup_hooks_.erase(cb);
31+
}
32+
}
33+
34+
size_t CleanupQueue::CleanupHookCallback::Hash::operator()(
35+
const CleanupHookCallback& cb) const {
36+
return std::hash<void*>()(cb.arg_);
37+
}
38+
39+
bool CleanupQueue::CleanupHookCallback::Equal::operator()(
40+
const CleanupHookCallback& a, const CleanupHookCallback& b) const {
41+
return a.fn_ == b.fn_ && a.arg_ == b.arg_;
42+
}
43+
44+
} // namespace node
Collapse file

‎src/cleanup_queue.h‎

Copy file name to clipboard
+83Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
#ifndef SRC_CLEANUP_QUEUE_H_
2+
#define SRC_CLEANUP_QUEUE_H_
3+
4+
#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
5+
6+
#include <cstddef>
7+
#include <cstdint>
8+
#include <unordered_set>
9+
10+
#include "memory_tracker.h"
11+
12+
namespace node {
13+
14+
class BaseObject;
15+
16+
class CleanupQueue : public MemoryRetainer {
17+
public:
18+
typedef void (*Callback)(void*);
19+
20+
CleanupQueue() {}
21+
22+
// Not copyable.
23+
CleanupQueue(const CleanupQueue&) = delete;
24+
25+
SET_MEMORY_INFO_NAME(CleanupQueue)
26+
inline void MemoryInfo(node::MemoryTracker* tracker) const override;
27+
inline size_t SelfSize() const override;
28+
29+
inline bool empty() const;
30+
31+
inline void Add(Callback cb, void* arg);
32+
inline void Remove(Callback cb, void* arg);
33+
void Drain();
34+
35+
template <typename T>
36+
inline void ForEachBaseObject(T&& iterator) const;
37+
38+
private:
39+
class CleanupHookCallback {
40+
public:
41+
CleanupHookCallback(Callback fn,
42+
void* arg,
43+
uint64_t insertion_order_counter)
44+
: fn_(fn),
45+
arg_(arg),
46+
insertion_order_counter_(insertion_order_counter) {}
47+
48+
// Only hashes `arg_`, since that is usually enough to identify the hook.
49+
struct Hash {
50+
size_t operator()(const CleanupHookCallback& cb) const;
51+
};
52+
53+
// Compares by `fn_` and `arg_` being equal.
54+
struct Equal {
55+
bool operator()(const CleanupHookCallback& a,
56+
const CleanupHookCallback& b) const;
57+
};
58+
59+
private:
60+
friend class CleanupQueue;
61+
Callback fn_;
62+
void* arg_;
63+
64+
// We keep track of the insertion order for these objects, so that we can
65+
// call the callbacks in reverse order when we are cleaning up.
66+
uint64_t insertion_order_counter_;
67+
};
68+
69+
inline BaseObject* GetBaseObject(const CleanupHookCallback& callback) const;
70+
71+
// Use an unordered_set, so that we have efficient insertion and removal.
72+
std::unordered_set<CleanupHookCallback,
73+
CleanupHookCallback::Hash,
74+
CleanupHookCallback::Equal>
75+
cleanup_hooks_;
76+
uint64_t cleanup_hook_counter_ = 0;
77+
};
78+
79+
} // namespace node
80+
81+
#endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
82+
83+
#endif // SRC_CLEANUP_QUEUE_H_
Collapse file

‎src/env-inl.h‎

Copy file name to clipboardExpand all lines: src/env-inl.h
+5-31Lines changed: 5 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -788,43 +788,17 @@ inline void Environment::ThrowUVException(int errorno,
788788
UVException(isolate(), errorno, syscall, message, path, dest));
789789
}
790790

791-
void Environment::AddCleanupHook(CleanupCallback fn, void* arg) {
792-
auto insertion_info = cleanup_hooks_.emplace(CleanupHookCallback {
793-
fn, arg, cleanup_hook_counter_++
794-
});
795-
// Make sure there was no existing element with these values.
796-
CHECK_EQ(insertion_info.second, true);
797-
}
798-
799-
void Environment::RemoveCleanupHook(CleanupCallback fn, void* arg) {
800-
CleanupHookCallback search { fn, arg, 0 };
801-
cleanup_hooks_.erase(search);
802-
}
803-
804-
size_t CleanupHookCallback::Hash::operator()(
805-
const CleanupHookCallback& cb) const {
806-
return std::hash<void*>()(cb.arg_);
791+
void Environment::AddCleanupHook(CleanupQueue::Callback fn, void* arg) {
792+
cleanup_queue_.Add(fn, arg);
807793
}
808794

809-
bool CleanupHookCallback::Equal::operator()(
810-
const CleanupHookCallback& a, const CleanupHookCallback& b) const {
811-
return a.fn_ == b.fn_ && a.arg_ == b.arg_;
812-
}
813-
814-
BaseObject* CleanupHookCallback::GetBaseObject() const {
815-
if (fn_ == BaseObject::DeleteMe)
816-
return static_cast<BaseObject*>(arg_);
817-
else
818-
return nullptr;
795+
void Environment::RemoveCleanupHook(CleanupQueue::Callback fn, void* arg) {
796+
cleanup_queue_.Remove(fn, arg);
819797
}
820798

821799
template <typename T>
822800
void Environment::ForEachBaseObject(T&& iterator) {
823-
for (const auto& hook : cleanup_hooks_) {
824-
BaseObject* obj = hook.GetBaseObject();
825-
if (obj != nullptr)
826-
iterator(obj);
827-
}
801+
cleanup_queue_.ForEachBaseObject(std::forward<T>(iterator));
828802
}
829803

830804
template <typename T>
Collapse file

‎src/env.cc‎

Copy file name to clipboardExpand all lines: src/env.cc
+4-31Lines changed: 4 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1022,33 +1022,10 @@ void Environment::RunCleanup() {
10221022
bindings_.clear();
10231023
CleanupHandles();
10241024

1025-
while (!cleanup_hooks_.empty() ||
1026-
native_immediates_.size() > 0 ||
1025+
while (!cleanup_queue_.empty() || native_immediates_.size() > 0 ||
10271026
native_immediates_threadsafe_.size() > 0 ||
10281027
native_immediates_interrupts_.size() > 0) {
1029-
// Copy into a vector, since we can't sort an unordered_set in-place.
1030-
std::vector<CleanupHookCallback> callbacks(
1031-
cleanup_hooks_.begin(), cleanup_hooks_.end());
1032-
// We can't erase the copied elements from `cleanup_hooks_` yet, because we
1033-
// need to be able to check whether they were un-scheduled by another hook.
1034-
1035-
std::sort(callbacks.begin(), callbacks.end(),
1036-
[](const CleanupHookCallback& a, const CleanupHookCallback& b) {
1037-
// Sort in descending order so that the most recently inserted callbacks
1038-
// are run first.
1039-
return a.insertion_order_counter_ > b.insertion_order_counter_;
1040-
});
1041-
1042-
for (const CleanupHookCallback& cb : callbacks) {
1043-
if (cleanup_hooks_.count(cb) == 0) {
1044-
// This hook was removed from the `cleanup_hooks_` set during another
1045-
// hook that was run earlier. Nothing to do here.
1046-
continue;
1047-
}
1048-
1049-
cb.fn_(cb.arg_);
1050-
cleanup_hooks_.erase(cb);
1051-
}
1028+
cleanup_queue_.Drain();
10521029
CleanupHandles();
10531030
}
10541031

@@ -1847,10 +1824,6 @@ void Environment::BuildEmbedderGraph(Isolate* isolate,
18471824
MemoryTracker tracker(isolate, graph);
18481825
Environment* env = static_cast<Environment*>(data);
18491826
tracker.Track(env);
1850-
env->ForEachBaseObject([&](BaseObject* obj) {
1851-
if (obj->IsDoneInitializing())
1852-
tracker.Track(obj);
1853-
});
18541827
}
18551828

18561829
size_t Environment::NearHeapLimitCallback(void* data,
@@ -1985,6 +1958,7 @@ inline size_t Environment::SelfSize() const {
19851958
// this can be done for common types within the Track* calls automatically
19861959
// if a certain scope is entered.
19871960
size -= sizeof(async_hooks_);
1961+
size -= sizeof(cleanup_queue_);
19881962
size -= sizeof(tick_info_);
19891963
size -= sizeof(immediate_info_);
19901964
return size;
@@ -2002,8 +1976,7 @@ void Environment::MemoryInfo(MemoryTracker* tracker) const {
20021976
tracker->TrackField("should_abort_on_uncaught_toggle",
20031977
should_abort_on_uncaught_toggle_);
20041978
tracker->TrackField("stream_base_state", stream_base_state_);
2005-
tracker->TrackFieldWithSize(
2006-
"cleanup_hooks", cleanup_hooks_.size() * sizeof(CleanupHookCallback));
1979+
tracker->TrackField("cleanup_queue", cleanup_queue_);
20071980
tracker->TrackField("async_hooks", async_hooks_);
20081981
tracker->TrackField("immediate_info", immediate_info_);
20091982
tracker->TrackField("tick_info", tick_info_);

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.