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 <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Juan José Arboleda <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>

Author: mhdawson

src/js_native_api_v8.cc

@@ -228,10 +228,9 @@ class RefBase : protected Finalizer, RefTracker {
   // from one of Unwrap or napi_delete_reference.
   //
   // When it is called from Unwrap or napi_delete_reference we only
-  // want to do the delete if there is no finalizer or the finalizer has already
-  // run or cannot have been queued to run (i.e. the reference count is > 0),
+  // want to do the delete if the finalizer has already run or
+  // cannot have been queued to run (ie the reference count is > 0),
   // otherwise we may crash when the finalizer does run.
-  //
   // If the finalizer may have been queued and has not already run
   // delay the delete until the finalizer runs by not doing the delete
   // and setting _delete_self to true so that the finalizer will
@@ -243,7 +242,6 @@ class RefBase : protected Finalizer, RefTracker {
   static inline void Delete(RefBase* reference) {
     reference->Unlink();
     if ((reference->RefCount() != 0) ||
-        (reference->_finalize_callback == nullptr) ||
         (reference->_delete_self) ||
         (reference->_finalize_ran)) {
       delete reference;

test/node-api/test_worker_terminate_finalization/test.js

@@ -1,6 +1,12 @@
 'use strict';
 const common = require('../../common');
 
+// TODO(addaleax): Run this test once it stops failing under ASAN/valgrind.
+// Refs: https://github.com/nodejs/node/issues/34731
+// Refs: https://github.com/nodejs/node/pull/35777
+// Refs: https://github.com/nodejs/node/issues/35778
+common.skip('Reference management in N-API leaks memory');
+
 const { Worker, isMainThread } = require('worker_threads');
 
 if (isMainThread) {