n-api: napi_wrap-ped finalize_cb not invoked

[legendecas]

• Version: 10.9.0
• Platform: macOS
• Subsystem: n-api

It seems napi_wraped object's finalize_cb is not called on test case: https://github.com/nodejs/node/blob/v10.9.0/test/addons-napi/7_factory_wrap/myobject.cc#L8

Yet if we wrap the JS code https://github.com/nodejs/node/blob/v10.9.0/test/addons-napi/7_factory_wrap/test.js#L6-L14 with a function context and force gc like following:

'use strict';
const common = require('../../common');
const assert = require('assert');
const createObject = require(`./build/${common.buildType}/binding`);

function context() {
  const obj = createObject(10);
  assert.strictEqual(obj.plusOne(), 11);
  assert.strictEqual(obj.plusOne(), 12);
  assert.strictEqual(obj.plusOne(), 13);

  const obj2 = createObject(20);
  assert.strictEqual(obj2.plusOne(), 21);
  assert.strictEqual(obj2.plusOne(), 22);
  assert.strictEqual(obj2.plusOne(), 23);
}
context();
global.gc();

the finalize_cb does be called.

[mhdawson]

I think you are suggesting we should update the test, I agree? Do you want to open a PR to do that?

[legendecas]

Yes, if possible. But I didn't figure out why in bare context objects' finalize callback was not invoked, since objects should be gc-ed on exit right?

[mhdawson]

I don't think there is any requirement for a gc on exit. Unless there is somewhere in the JS spec that says finalizers will be invoked before exit (my guess is that there is not) then the process can simply exist and all memory is freed so no reason to run a gc first.

[legendecas]

Sounds reasonable to me that there is no reason to run gc first before exit.

More question: whether finalize callback shall be ensured to be invoked in this test case? IMO there are reasons to test it to cover napi wraps.

[mhdawson]

It would be would to have the test case cover the callback being invoked if possible. Do you want to update PR to update the test to validate that?

[legendecas]

@mhdawson PR is submitted :p

[gabrielschulhof]

@legendecas the finalizer will be called if you do

let obj1 = createObject(10);
obj1 = null;
global.gc();

so this is not actually a bug.

[gabrielschulhof]

@legendecas we actually check exactly that the finalizer gets called in https://github.com/nodejs/node/blob/master/test/addons-napi/8_passing_wrapped/test.js.