Commit Graph

7 Commits (2aff5cf638dcfbf89cf82eb42e6fe6d03cf75000)

Author SHA1 Message Date
Gabriel Schulhof 4b7f23f868 test: rename n-api to node-api
This renames the macros used in the tests from `NAPI_*` to
`NODE_API_*`.

PR-URL: https://github.com/nodejs/node/pull/37217
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Beth Griggs <bgriggs@redhat.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
2021-02-06 05:03:38 -08:00
Gabriel Schulhof c822ba7121 n-api: unlink reference during its destructor
Currently, a reference is being unlinked from the list of references
tracked by the environment when `v8impl::Reference::Delete` is called.
This causes a leak when deletion must be deferred because the finalizer
hasn't yet run, but the finalizer does not run because environment
teardown is in progress, and so no more gc runs will happen, and the
`FinalizeAll` run that happens during environment teardown does not
catch the reference because it's no longer in the list. The test below
will fail when running with ASAN:

```
./node ./test/node-api/test_worker_terminate_finalization/test.js
```

OTOH if, to address the above leak, we make a special case to not
unlink a reference during environment teardown, we run into a
situation where the reference gets deleted by
`v8impl::Reference::Delete` but does not get unlinked because it's
environment teardown time. This leaves a stale pointer in the linked
list which will result in a use-after-free in `FinalizeAll` during
environment teardown. The test below will fail if we make the above
change:

```
./node -e "require('./test/node-api/test_instance_data/build/Release/test_ref_then_set.node');"
```

Thus, we unlink a reference precisely when we destroy it – in its
destructor.

Refs: https://github.com/nodejs/node/issues/34731
Refs: https://github.com/nodejs/node/pull/34839
Refs: https://github.com/nodejs/node/issues/35620
Refs: https://github.com/nodejs/node/issues/35777
Fixes: https://github.com/nodejs/node/issues/35778
Signed-off-by: Gabriel Schulhof <gabriel.schulhof@intel.com>
PR-URL: https://github.com/nodejs/node/pull/35933
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Zeyu Yang <himself65@outlook.com>
2020-11-04 22:00:42 -08:00
Michael Dawson 0b45382dff n-api: revert change to finalization
Fixes: https://github.com/nodejs/node/issues/35620

This reverts commit a6b655614f which
changed finalization behavior related to N-API. We will investigate
the original issue with the test separately.

PR-URL: https://github.com/nodejs/node/pull/35777
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
2020-10-27 11:40:33 +01:00
Gabriel Schulhof acd423b45e n-api: handle weak no-finalizer refs correctly
When deleting a weak reference that has no finalizer we must not defer
deletion until the non-existent finalizer gets called.

Fixes: https://github.com/nodejs/node/issues/34731
Signed-off-by: Gabriel Schulhof <gabriel.schulhof@intel.com>

PR-URL: https://github.com/nodejs/node/pull/34839
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
2020-08-21 06:16:18 -07:00
Anna Henningsen cf5d7c0140
test: skip node-api/test_worker_terminate_finalization
The test fails under ASAN/valgrind. Since it has not been working
properly until today anyway, skip it.

Refs: https://github.com/nodejs/node/issues/34731

PR-URL: https://github.com/nodejs/node/pull/34732
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matheus Marchini <mat@mmarchini.me>
2020-08-11 18:36:51 -07:00
Anna Henningsen 9f47e5a7f7
test: fix test_worker_terminate_finalization
The test was missing an initialization of the global `ref` variable
because there was also an unused local one, leading to failures
like the one seen in https://github.com/nodejs/node/pull/34625.

PR-URL: https://github.com/nodejs/node/pull/34726
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ricky Zhou <0x19951125@gmail.com>
2020-08-11 18:04:58 +02:00
Michael Dawson 362e4a1aec n-api: ensure scope present for finalization
Refs: https://github.com/nodejs/node-addon-api/issues/722

Ensure a scope is on stack during finalization
as finalization functions can create JS Objects

Signed-off-by: Michael Dawson <michael_dawson@ca.ibm.com>

PR-URL: https://github.com/nodejs/node/pull/33508
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
2020-06-09 19:17:49 -04:00