n-api: keep napi_env alive while it has finalizers

[addaleax]

Manage the napi_env refcount from Finalizer instances, as the
finalizer may refer to the napi_env until it is deleted.

Fixes: #31134
Fixes: node-ffi-napi/node-ffi-napi#48

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• tests and/or benchmarks are included
• commit message follows commit guidelines

[nodejs-github-bot]

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

test failures

[nodejs-github-bot]

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

[legendecas]

Just out of curiosity, why wrapped objects by napi_wrap do not surface this issue? I tested against objects been wrapped which has been attached to process object too, it didn't crash on exit.

[addaleax]

@legendecas Because those finalizers are called during the destruction of napi_env, so the napi_env is still accessible at that point, as I understand it.

[nodejs-github-bot]

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

[legendecas]

@addaleax Because those finalizers are called during the destruction of napi_env, so the napi_env is still accessible at that point, as I understand it.

Yeah, there are two lists tracking these references. Wondering if it is possible to track those array buffers in the list too?

[addaleax]

@addaleax Because those finalizers are called during the destruction of napi_env, so the napi_env is still accessible at that point, as I understand it.

Yeah, there are two lists tracking these references. Wondering if it is possible to track those array buffers in the list too?

Well … In an ideal world, I think it would have been cleaner and more consistent for N-API not to just blindly wrap the Node.js C++ Buffer API, and instead only provide the option to create a Buffer using napi_create_typedarray(env, napi_buffer, ...) analogously to napi_create_typedarray(env, napi_uint8_array, ...).

However, the way things currently work is that the N-API methods for dealing with buffers correspond to the C++ Buffer API. That has its own finalization mechanism.

The only way “out” of this that I could see is to try and implement the N-API buffer methods in terms of the regular N-API typed array methods – feel free to do that if you like, but it seems like that’s out of scope for this PR.

[nodejs-github-bot]

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

[nodejs-github-bot]

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

[addaleax]

Landed in 493faf6