delta/node-new.git/test/node-api, branch devsnek-patch-1

n-api: unlink reference during its destructor

2020-11-05T06:00:42+00:00

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 
PR-URL: https://github.com/nodejs/node/pull/35933
Reviewed-By: Rich Trott 
Reviewed-By: Michael Dawson 
Reviewed-By: Zeyu Yang

n-api: napi_make_callback emit async init with resource of async_context

2020-10-31T00:17:59+00:00

instead of emit async init with receiver of the callback.

PR-URL: https://github.com/nodejs/node/pull/32930
Fixes: https://github.com/nodejs/node/issues/32898
Reviewed-By: Stephen Belanger 
Reviewed-By: Michael Dawson 
Reviewed-By: Gerhard Stöbich

n-api: revert change to finalization

2020-10-27T10:40:33+00:00

Fixes: https://github.com/nodejs/node/issues/35620

This reverts commit a6b655614f03e073b9c60f3d71ed884c5af32ffc 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 
Reviewed-By: Rich Trott 
Reviewed-By: Juan José Arboleda 
Reviewed-By: Chengzhong Wu 
Reviewed-By: Gerhard Stöbich

test: adjust comments for upcoming lint rule

2020-10-07T11:36:02+00:00

Enforce `//` for multiline comments. Some tests mixed and matched, and
at least one did so in a (to me) surprising way.

PR-URL: https://github.com/nodejs/node/pull/35485
Reviewed-By: Colin Ihrig 
Reviewed-By: James M Snell

n-api: re-implement async env cleanup hooks

2020-08-27T15:18:37+00:00

* Avoid passing core `void*` and function pointers into add-on.
* Document `napi_async_cleanup_hook_handle` type.
* Render receipt of the handle mandatory from the point where the
  hook gets called. Removal of the handle remains mandatory.

Fixes: https://github.com/nodejs/node/issues/34715
Signed-off-by: Gabriel Schulhof 
Co-authored-by: Anna Henningsen 
PR-URL: https://github.com/nodejs/node/pull/34819
Reviewed-By: Michael Dawson 
Reviewed-By: Zeyu Yang

n-api: handle weak no-finalizer refs correctly

2020-08-21T13:16:18+00:00

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 

PR-URL: https://github.com/nodejs/node/pull/34839
Reviewed-By: Anna Henningsen 
Reviewed-By: Gus Caplan 
Reviewed-By: James M Snell 
Reviewed-By: Rich Trott

test: skip node-api/test_worker_terminate_finalization

2020-08-12T01:36:51+00:00

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 
Reviewed-By: Matheus Marchini

test: fix test_worker_terminate_finalization

2020-08-11T16:04:58+00:00

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 
Reviewed-By: James M Snell 
Reviewed-By: Ricky Zhou <0x19951125@gmail.com>

n-api,src: provide asynchronous cleanup hooks

2020-08-06T22:02:02+00:00

Sometimes addons need to perform cleanup actions, for example
closing libuv handles or waiting for requests to finish, that
cannot be performed synchronously.

Add C++ API and N-API functions that allow providing such
asynchronous cleanup hooks.

Fixes: https://github.com/nodejs/node/issues/34567

PR-URL: https://github.com/nodejs/node/pull/34572
Reviewed-By: James M Snell 
Reviewed-By: Gabriel Schulhof

test: convert most N-API tests from C++ to C

2020-08-05T21:21:51+00:00

* Prefix functions with `static` to make them local
* Remove anonymous namespaces
* `nullptr` -> `NULL`
* .cc -> .c and update binding.gyp
* `static_cast()` -> `(x)()`
* Replace `new`/`delete` with `malloc()`/`free()`
  (only in test_callback_scope)
* Move lambda out and convert to local function
  (only in test_callback_scope)
* Remove superfluous `#include `
  (only in test_callback_scope_recurse)

Some tests are best left as C++.

```bash
ls -l test/{node-api,js-native-api}/*/*.cc
```

for those remaining as C++ tests.

Signed-off-by: Gabriel Schulhof 
PR-URL: https://github.com/nodejs/node/pull/34615
Reviewed-By: James M Snell 
Reviewed-By: Anna Henningsen 
Reviewed-By: Jiawen Geng 
Reviewed-By: Joyee Cheung 
Reviewed-By: Gerhard Stöbich