src: close HandleWraps instead of deleting them in OnGCCollect() · nodejs/node@e6ff7e6 · GitHub

nodejs / node Public

• Notifications You must be signed in to change notification settings
• Fork 36.6k
• Star 120k

• Code
• Issues 635
• Pull requests 755
• Actions
• Projects
• Security and quality
• Insights

Additional navigation options

Commit e6ff7e6

Browse filesBrowse the repository at this point in the historyBrowse files

addaleax

authored and

targos

committed

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
Copy full SHA for e6ff7e6

2 files changed

+11-2Lines changed: 11 additions & 2 deletions

File tree

Expand file treeCollapse file tree

Open diff view settings

Filter options

• src
  • base_object-inl.h
  • handle_wrap.cc

Expand file treeCollapse file tree

Open diff view settings

Collapse file

‎src/base_object-inl.h‎

Copy file name to clipboardExpand all lines: src/base_object-inl.h

+1-1Lines changed: 1 addition & 1 deletion

Original file line number	Diff line number	Diff line change
@@ -207,7 +207,7 @@ void BaseObject::decrease_refcount() {
207	207	unsigned int new_refcount = --metadata->strong_ptr_count;
208	208	if (new_refcount == 0) {
209	209	if (metadata->is_detached) {
210		- delete this;
	210	+ OnGCCollect();
211	211	} else if (metadata->wants_weak_jsobj && !persistent_handle_.IsEmpty()) {
212	212	MakeWeak();
213	213	}

Collapse file

‎src/handle_wrap.cc‎

Copy file name to clipboardExpand all lines: src/handle_wrap.cc

+10-1Lines changed: 10 additions & 1 deletion

Original file line number	Diff line number	Diff line change
@@ -84,7 +84,16 @@ void HandleWrap::Close(Local<Value> close_callback) {
84	84
85	85
86	86	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	+ }
88	97	}
89	98
90	99

0 commit comments

Comments

0 (0)