src: consolidate environment cleanup queue · nodejs/node@e8101af · GitHub
Skip to content

Commit e8101af

Browse 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

10 files changed

Lines changed: 219 additions & 106 deletions

File tree

‎node.gyp‎

Lines changed: 3 additions & 0 deletions

‎src/base_object.h‎

Lines 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

‎src/cleanup_queue-inl.h‎

Lines 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_

‎src/cleanup_queue.cc‎

Lines 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

‎src/cleanup_queue.h‎

Lines 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_

‎src/env-inl.h‎

Lines 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>

‎src/env.cc‎

Lines changed: 4 additions & 31 deletions

0 commit comments

Comments
 (0)