release non-object types from napi_ref on ref count 0

doc/api/n-api.md

@@ -1654,10 +1654,11 @@ method.
 Node-API provides methods to create persistent references to values.
 Each reference has an associated count with a value of 0 or higher.
 The count determines if the reference will keep the corresponding value alive.
-References with a count of 0 do not prevent values of object types (object,
-function, external) from being collected and are often called 'weak'
-references. Values of non-object types cannot be collected if a reference
-has count 0 because they do not support the 'weak' object semantic.
+References with a count of 0 do not prevent values from being collected.
+Values of object types (object, function, external) are becoming 'weak'
+references and can still be accessed while they are not collected.
+Values of non-object types are released when the count becomes 0
+and cannot be accessed from the reference any more.
 Any count greater than 0 will prevent the values from being collected.
 
 References can be created with an initial reference count. The count can

src/js_native_api_v8.cc

@@ -551,7 +551,8 @@ void RefBase::Finalize() {
 template <typename... Args>
 Reference::Reference(napi_env env, v8::Local<v8::Value> value, Args&&... args)
     : RefBase(env, std::forward<Args>(args)...),
-      persistent_(env->isolate, value) {
+      persistent_(env->isolate, value),
+      can_be_weak_(value->IsObject()) {
   if (RefCount() == 0) {
     SetWeak();
   }
@@ -585,7 +586,7 @@ uint32_t Reference::Ref() {
     return 0;
   }
   uint32_t refcount = RefBase::Ref();
-  if (refcount == 1) {
+  if (refcount == 1 && can_be_weak_) {
     persistent_.ClearWeak();
   }
   return refcount;
@@ -625,7 +626,11 @@ void Reference::Finalize() {
 // Mark the reference as weak and eligible for collection
 // by the gc.
 void Reference::SetWeak() {
-  persistent_.SetWeak(this, WeakCallback, v8::WeakCallbackType::kParameter);
+  if (can_be_weak_) {
+    persistent_.SetWeak(this, WeakCallback, v8::WeakCallbackType::kParameter);
+  } else {
+    persistent_.Reset();
+  }
 }
 
 // The N-API finalizer callback may make calls into the engine. V8's heap is

src/js_native_api_v8.h

@@ -425,6 +425,7 @@ class Reference : public RefBase {
   void SetWeak();
 
   v8impl::Persistent<v8::Value> persistent_;
+  bool can_be_weak_;
 };
 
 }  // end of namespace v8impl

test/js-native-api/test_reference/test.js

@@ -17,21 +17,28 @@ async function runTests() {
   (() => {
     const symbol = test_reference.createSymbol('testSym');
     test_reference.createReference(symbol, 0);
+    assert.strictEqual(test_reference.referenceValue, undefined);
+  })();
+  test_reference.deleteReference();
+
+  (() => {
+    const symbol = test_reference.createSymbol('testSym');
+    test_reference.createReference(symbol, 1);
     assert.strictEqual(test_reference.referenceValue, symbol);
   })();
   test_reference.deleteReference();
 
   (() => {
     const symbol = test_reference.createSymbolFor('testSymFor');
-    test_reference.createReference(symbol, 0);
+    test_reference.createReference(symbol, 1);
     assert.strictEqual(test_reference.referenceValue, symbol);
     assert.strictEqual(test_reference.referenceValue, Symbol.for('testSymFor'));
   })();
   test_reference.deleteReference();
 
   (() => {
     const symbol = test_reference.createSymbolForEmptyString();
-    test_reference.createReference(symbol, 0);
+    test_reference.createReference(symbol, 1);
     assert.strictEqual(test_reference.referenceValue, symbol);
     assert.strictEqual(test_reference.referenceValue, Symbol.for(''));
   })();

test/node-api/test_reference_by_node_api_version/test.js

@@ -60,12 +60,17 @@ async function runTests(addon, isVersion8) {
       }
     }
 
-    // The references become weak pointers when the ref count is 0.
+    // When the reference count is zero, then object types become weak pointers
+    // and other types are released.
     // Here we know that the GC is not run yet because the values are
     // still in the allEntries array.
     allEntries.forEach((entry, index) => {
       if (!isVersion8 || entry.canBeRefV8) {
-        assert.strictEqual(addon.getRefValue(index), entry.value);
+        if (entry.canBeWeak) {
+          assert.strictEqual(addon.getRefValue(index), entry.value);
+        } else {
+          assert.strictEqual(addon.getRefValue(index), undefined);
+        }
       }
       // Set to undefined to allow GC collect the value.
       entry.value = undefined;
@@ -90,15 +95,9 @@ async function runTests(addon, isVersion8) {
 
   // After GC and finalizers run, all values that support weak reference
   // semantic must return undefined value.
-  // It also includes the value at index 0 because it is the undefined value.
-  // Other value types are not collected by GC.
   allEntries.forEach((entry, index) => {
     if (!isVersion8 || entry.canBeRefV8) {
-      if (entry.canBeWeak || index === 0) {
-        assert.strictEqual(addon.getRefValue(index), undefined);
-      } else {
-        assert.notStrictEqual(addon.getRefValue(index), undefined);
-      }
+      assert.strictEqual(addon.getRefValue(index), undefined);
       addon.deleteRef(index);
     }
   });