Skip to content

Commit e6ff7e6

Browse files
addaleaxtargos
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 <tniessen@tnie.de> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent d863a9d commit e6ff7e6

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
@@ -207,7 +207,7 @@ void BaseObject::decrease_refcount() {
207207
unsigned int new_refcount = --metadata->strong_ptr_count;
208208
if (new_refcount == 0) {
209209
if (metadata->is_detached) {
210-
delete this;
210+
OnGCCollect();
211211
} else if (metadata->wants_weak_jsobj && !persistent_handle_.IsEmpty()) {
212212
MakeWeak();
213213
}

src/handle_wrap.cc

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

8585

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

9099

0 commit comments

Comments
 (0)