Skip to content

Commit f0287e5

Browse files
addaleaxBethGriggs
authored andcommitted
src: close HandleWraps instead of deleting them in OnGCCollect()
When all strong `BaseObjectPtr`s to a `HandleWrap` are gone, we should not delete the `HandleWrap` outright, but instead close it and then delete it only once the libuv close callback has been called. Based on the valgrind output from the issue below, this has a good chance of fixing it. Fixes: #39036 PR-URL: #39441 Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
1 parent 89796d0 commit f0287e5

File tree

2 files changed

+11
-2
lines changed

2 files changed

+11
-2
lines changed

src/base_object-inl.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,7 @@ void BaseObject::decrease_refcount() {
201201
unsigned int new_refcount = --metadata->strong_ptr_count;
202202
if (new_refcount == 0) {
203203
if (metadata->is_detached) {
204-
delete this;
204+
OnGCCollect();
205205
} else if (metadata->wants_weak_jsobj && !persistent_handle_.IsEmpty()) {
206206
MakeWeak();
207207
}

src/handle_wrap.cc

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,16 @@ void HandleWrap::Close(Local<Value> close_callback) {
8585

8686

8787
void HandleWrap::OnGCCollect() {
88-
Close();
88+
// When all references to a HandleWrap are lost and the object is supposed to
89+
// be destroyed, we first call Close() to clean up the underlying libuv
90+
// handle. The OnClose callback then acquires and destroys another reference
91+
// to that object, and when that reference is lost, we perform the default
92+
// action (i.e. destroying `this`).
93+
if (state_ != kClosed) {
94+
Close();
95+
} else {
96+
BaseObject::OnGCCollect();
97+
}
8998
}
9099

91100

0 commit comments

Comments
 (0)