Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit a6b6556

Browse filesBrowse files
Gabriel Schulhofaddaleax
authored andcommitted
n-api: handle weak no-finalizer refs correctly
When deleting a weak reference that has no finalizer we must not defer deletion until the non-existent finalizer gets called. Fixes: #34731 Signed-off-by: Gabriel Schulhof <gabriel.schulhof@intel.com> PR-URL: #34839 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
1 parent f87b6c0 commit a6b6556
Copy full SHA for a6b6556

File tree

Expand file treeCollapse file tree

2 files changed

+4
-6
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

2 files changed

+4
-6
lines changed
Open diff view settings
Collapse file

‎src/js_native_api_v8.cc‎

Copy file name to clipboardExpand all lines: src/js_native_api_v8.cc
+4-2Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -225,9 +225,10 @@ class RefBase : protected Finalizer, RefTracker {
225225
// from one of Unwrap or napi_delete_reference.
226226
//
227227
// When it is called from Unwrap or napi_delete_reference we only
228-
// want to do the delete if the finalizer has already run or
229-
// cannot have been queued to run (ie the reference count is > 0),
228+
// want to do the delete if there is no finalizer or the finalizer has already
229+
// run or cannot have been queued to run (i.e. the reference count is > 0),
230230
// otherwise we may crash when the finalizer does run.
231+
//
231232
// If the finalizer may have been queued and has not already run
232233
// delay the delete until the finalizer runs by not doing the delete
233234
// and setting _delete_self to true so that the finalizer will
@@ -239,6 +240,7 @@ class RefBase : protected Finalizer, RefTracker {
239240
static inline void Delete(RefBase* reference) {
240241
reference->Unlink();
241242
if ((reference->RefCount() != 0) ||
243+
(reference->_finalize_callback == nullptr) ||
242244
(reference->_delete_self) ||
243245
(reference->_finalize_ran)) {
244246
delete reference;
Collapse file

‎test/node-api/test_worker_terminate_finalization/test.js‎

Copy file name to clipboardExpand all lines: test/node-api/test_worker_terminate_finalization/test.js
-4Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,6 @@
11
'use strict';
22
const common = require('../../common');
33

4-
// TODO(addaleax): Run this test once it stops failing under ASAN/valgrind.
5-
// Refs: https://github.com/nodejs/node/issues/34731
6-
common.skip('Reference management in N-API leaks memory');
7-
84
const { Worker, isMainThread } = require('worker_threads');
95

106
if (isMainThread) {

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.