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

nodejs / node Public

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

• Code
• Issues 633
• Pull requests 591
• Actions
• Projects
• Security and quality
• Insights

Additional navigation options

Commit f0287e5

Browse filesBrowse the repository at this point in the historyBrowse files

addaleax

authored and

BethGriggs

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 89796d0 commit f0287e5
Copy full SHA for f0287e5

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
@@ -201,7 +201,7 @@ void BaseObject::decrease_refcount() {
201	201	unsigned int new_refcount = --metadata->strong_ptr_count;
202	202	if (new_refcount == 0) {
203	203	if (metadata->is_detached) {
204		- delete this;
	204	+ OnGCCollect();
205	205	} else if (metadata->wants_weak_jsobj && !persistent_handle_.IsEmpty()) {
206	206	MakeWeak();
207	207	}

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
@@ -85,7 +85,16 @@ void HandleWrap::Close(Local<Value> close_callback) {
85	85
86	86
87	87	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	+ }
89	98	}
90	99
91	100

0 commit comments

Comments

0 (0)