node-api: add nested object wrap and napi_ref test

[legendecas]

Test that an napi_ref can be nested inside another ObjectWrap. The test
shows a critical case where a finalizer deletes an napi_ref whose
finalizer is also scheduled.

This add a test for comments at:

node/src/node_api.cc

Lines 116 to 118 in 68cc1c9

	// As userland code can delete additional references in one finalizer,
	// the list of pending finalizers may be mutated as we execute them, so
	// we keep iterating it until it is empty.

The changes in test/js-native-api/6_object_wrap/myobject.cc are auto-formatted by make format-cpp as the file is renamed.

Refs: #57861 (review)

[nodejs-github-bot]

Review requested:

• @nodejs/gyp
• @nodejs/node-api

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.27%. Comparing base (e9b286c) to head (21ac07e).
Report is 26 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #57981      +/-   ##
==========================================
- Coverage   90.27%   90.27%   -0.01%     
==========================================
  Files         630      630              
  Lines      186165   186158       -7     
  Branches    36485    36475      -10     
==========================================
- Hits       168064   168049      -15     
- Misses      10974    10979       +5     
- Partials     7127     7130       +3     

see 35 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[vmoroz]

@legendecas , in regards to the PR #57861, do we ever have a situation when a napi_ref is removed from the finalizer queue, or added to it twice? I guess, in such cases the batched processing of the queue may not work.

[mhdawson]

LGTM

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/66428/

[legendecas]

@vmoroz in regards to the PR #57861, do we ever have a situation when a napi_ref is removed from the finalizer queue, or added to it twice? I guess, in such cases the batched processing of the queue may not work.

I think node_api_post_finalizer is the only node-api that allows putting a finalizer into the back of the queue, and it is how it supposed to work.

Putting a single finalizer into the queue twice is a bug. Do you have a minimum test to reproduce it?

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/66454/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/66455/

[mhdawson]

LGTM

[vmoroz]

@vmoroz in regards to the PR #57861, do we ever have a situation when a napi_ref is removed from the finalizer queue, or added to it twice? I guess, in such cases the batched processing of the queue may not work.

I think node_api_post_finalizer is the only node-api that allows putting a finalizer into the back of the queue, and it is how it supposed to work.

Putting a single finalizer into the queue twice is a bug. Do you have a minimum test to reproduce it?

@legendecas , no, I do not have a repro. I am just trying to understand if the PR #57861 has issues or not. Currently based on your answers, I tend to believe that it does not have issues. I wonder if you already saw any issues by running that PR against these new tests.

[legendecas]

@vmoroz I commented at #57861 (comment). The change in that PR crashes this new test.

[nodejs-github-bot]

Landed in 3b90f34