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 a93ca2d

Browse filesBrowse files
mhdawsonMylesBorins
authored andcommitted
n-api: revert change to finalization
Fixes: #35620 This reverts commit a6b6556 which changed finalization behavior related to N-API. We will investigate the original issue with the test separately. PR-URL: #35777 Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
1 parent df52814 commit a93ca2d
Copy full SHA for a93ca2d

File tree

Expand file treeCollapse file tree

2 files changed

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

2 files changed

+8
-4
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
+2-4Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -228,10 +228,9 @@ class RefBase : protected Finalizer, RefTracker {
228228
// from one of Unwrap or napi_delete_reference.
229229
//
230230
// When it is called from Unwrap or napi_delete_reference we only
231-
// want to do the delete if there is no finalizer or the finalizer has already
232-
// run or cannot have been queued to run (i.e. the reference count is > 0),
231+
// want to do the delete if the finalizer has already run or
232+
// cannot have been queued to run (ie the reference count is > 0),
233233
// otherwise we may crash when the finalizer does run.
234-
//
235234
// If the finalizer may have been queued and has not already run
236235
// delay the delete until the finalizer runs by not doing the delete
237236
// and setting _delete_self to true so that the finalizer will
@@ -243,7 +242,6 @@ class RefBase : protected Finalizer, RefTracker {
243242
static inline void Delete(RefBase* reference) {
244243
reference->Unlink();
245244
if ((reference->RefCount() != 0) ||
246-
(reference->_finalize_callback == nullptr) ||
247245
(reference->_delete_self) ||
248246
(reference->_finalize_ran)) {
249247
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
+6Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,12 @@
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+
// Refs: https://github.com/nodejs/node/pull/35777
7+
// Refs: https://github.com/nodejs/node/issues/35778
8+
common.skip('Reference management in N-API leaks memory');
9+
410
const { Worker, isMainThread } = require('worker_threads');
511

612
if (isMainThread) {

0 commit comments

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