Commit Graph

68 Commits

Author SHA1 Message Date
Antoine du Hamel
d640feee85 test: add trailing commas in test/node-api
PR-URL: https://github.com/nodejs/node/pull/46384
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
2023-01-29 20:01:09 +01:00
ywave620
8438f3b73b
process,worker: ensure code after exit() effectless
Cope with the delay(to the next function call) of
v8::Isolate::TerminateExecution()

PR-URL: https://github.com/nodejs/node/pull/45620
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
2022-12-25 09:54:12 +00:00
Michael Dawson
09ae62b9a8 node-api: handle no support for external buffers
Refs: https://github.com/electron/electron/issues/35801
Refs: https://github.com/nodejs/abi-stable-node/issues/441

Electron recently dropped support for external
buffers. Provide a way for addon authors to:
- hide the methods to create external buffers so they can
  avoid using them if they want the broadest compatibility.
- call the methods that create external buffers at runtime
  to check if external buffers are supported and either
  use them or not based on the return code.

Signed-off-by: Michael Dawson <mdawson@devrus.com>

PR-URL: https://github.com/nodejs/node/pull/45181
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
2022-11-09 13:30:43 -05:00
Santiago Gimeno
09cbb66f04
test: use uv_sleep() where possible
PR-URL: https://github.com/nodejs/node/pull/45124
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: theanarkh <theratliter@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Daeyeon Jeong <daeyeon.dev@gmail.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
2022-10-27 17:13:50 +00:00
Joyee Cheung
472edc775d
src: disambiguate terms used to refer to builtins and addons
The term "native module" dates back to some of the oldest code
in the code base. Within the context of Node.js core it usually
refers to modules that are native to Node.js (e.g. fs, http),
but it can cause confusion for people who don't work on this
part of the code base, as "native module" can also refer to
native addons - which is even the case in some of the API
docs and error messages.

This patch tries to make the usage of these terms more consistent.
Now within the context of Node.js core:

- JavaScript scripts that are built-in to Node.js are now referred
  to as "built-in(s)". If they are available as modules,
  they can also be referred to as "built-in module(s)".
- Dynamically-linked shared objects that are loaded into
  the Node.js processes are referred to as "addons".

We will try to avoid using the term "native modules" because it could
be ambiguous.

Changes in this patch:

File names:
- node_native_module.h -> node_builtins.h,
- node_native_module.cc -> node_builtins.cc

C++ binding names:
- `native_module` -> `builtins`

`node::Environment`:
- `native_modules_without_cache` -> `builtins_without_cache`
- `native_modules_with_cache` -> `builtins_with_cache`
- `native_modules_in_snapshot` -> `builtins_in_cache`
- `native_module_require` -> `builtin_module_require`

`node::EnvSerializeInfo`:
- `native_modules` -> `builtins

`node::native_module::NativeModuleLoader`:
- `native_module` namespace -> `builtins` namespace
- `NativeModuleLoader` -> `BuiltinLoader`
- `NativeModuleRecordMap` -> `BuiltinSourceMap`
- `NativeModuleCacheMap` -> `BuiltinCodeCacheMap`
- `ModuleIds` -> `BuiltinIds`
- `ModuleCategories` -> `BuiltinCategories`
- `LoadBuiltinModuleSource` -> `LoadBuiltinSource`

`loader.js`:
- `NativeModule` -> `BuiltinModule` (the `NativeModule` name used in
  `process.moduleLoadList` is kept for compatibility)

And other clarifications in the documentation and comments.

PR-URL: https://github.com/nodejs/node/pull/44135
Fixes: https://github.com/nodejs/node/issues/44036
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Jan Krems <jan.krems@gmail.com>
2022-08-09 01:36:49 +08:00
Feng Yu
ea99431586
test: mark flaky tests on smartos
PR-URL: https://github.com/nodejs/node/pull/43596
Refs: https://github.com/nodejs/node/issues/43457
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
2022-06-28 18:06:48 +01:00
Tobias Nießen
f5ce6b1ebf
test: replace gc(true) with gc({ type: 'minor' })
V8 considers gc(true) legacy, and the new signature is much more
expressive.

PR-URL: https://github.com/nodejs/node/pull/43493
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
2022-06-21 22:54:09 +01:00
Matteo Collina
562bf8bf5f Revert "test: mark test_buffer/test_finalizer flaky"
This reverts commit 73d8db896e.

PR-URL: https://github.com/nodejs/node/pull/43418
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
2022-06-17 08:32:57 +00:00
Matteo Collina
8bff086090 test: make node-api/test_buffer/test_finalizer not flaky
PR-URL: https://github.com/nodejs/node/pull/43418
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
2022-06-17 08:32:56 +00:00
Michael Dawson
73d8db896e test: mark test_buffer/test_finalizer flaky
Refs: https://github.com/nodejs/node/issues/43389

Mark new test as flaky as it seems to have
had intermittent failures since it was added.

Signed-off-by: Michael Dawson <mdawson@devrus.com>

PR-URL: https://github.com/nodejs/node/pull/43414
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
2022-06-14 09:52:55 -04:00
legendecas
681fb3a200 node-api: emit uncaught-exception on unhandled tsfn callbacks
PR-URL: https://github.com/nodejs/node/pull/36510
Fixes: https://github.com/nodejs/node/issues/36402
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
2022-05-27 13:32:08 -04:00
Vladimir Morozov
44fdf953ba node-api,src: fix module registration in MSVC C++
PR-URL: https://github.com/nodejs/node/pull/42459
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
2022-04-07 09:40:07 -04:00
Anna Henningsen
64832c12b1
src: return proper URLs from node_api_get_module_file_name
Using `file://${path}` does not properly escape special URL characters.

PR-URL: https://github.com/nodejs/node/pull/41758
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
2022-03-07 15:57:54 +01:00
Rich Trott
5d559f4a74 test: enable no-empty ESLint rule
PR-URL: https://github.com/nodejs/node/pull/41831
Refs: https://eslint.org/docs/rules/no-empty
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
2022-02-05 12:58:32 +00:00
Anna Henningsen
9f7412a6cd Revert "src: skip test_fatal/test_threads for Debug builds"
This reverts commit 1fc4d43a32.

PR-URL: https://github.com/nodejs/node/pull/39954
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
2021-09-08 18:19:47 +00:00
legendecas
99a3d55784
report: generates report on threads with no isolates
PR-URL: https://github.com/nodejs/node/pull/38994
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
2021-06-14 08:10:54 -07:00
Daniel Bevenius
1fc4d43a32 src: skip test_fatal/test_threads for Debug builds
Currently test/node-api/test_fatal/test_threads.js fails for a Debug
build with the following error:
 1: 0x101e3f8 node::DumpBacktrace(_IO_FILE*) [/node/out/Debug/node]
 2: 0x11c31ed  [/node/out/Debug/node]
 3: 0x11c320d  [/node/out/Debug/node]
 4: 0x2ba4448 V8_Fatal(char const*, int, char const*, ...) [/node/out/Debug/node]
 5: 0x2ba4473  [/node/out/Debug/node]
 6: 0x139e049 v8::internal::Isolate::Current() [/node/out/Debug/node]
 7: 0x11025ee node::OnFatalError(char const*, char const*) [/node/out/Debug/node]
 8: 0x1102564 node::FatalError(char const*, char const*) [/node/out/Debug/node]
 9: 0x10add1d napi_open_callback_scope [/node/out/Debug/node]
10: 0x7f05664211dc  [/node/test/node-api/test_fatal/build/Debug/test_fatal.node]
11: 0x7f056608e4e2  [/usr/lib64/libpthread.so.0]
12: 0x7f0565fbd6c3 clone [/usr/lib64/libc.so.6]

node:assert:412
    throw err;
    ^

AssertionError [ERR_ASSERTION]: The expression evaluated to a falsy value:

  assert.ok(p.status === 134 || p.signal === 'SIGABRT')

    at Object.<anonymous> (/node/test/node-api/test_fatal/test_threads.js:21:8)
    at Module._compile (node:internal/modules/cjs/loader:1109:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1138:10)
    at Module.load (node:internal/modules/cjs/loader:989:32)
    at Function.Module._load (node:internal/modules/cjs/loader:829:14)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:79:12)
    at node:internal/main/run_main_module:17:47 {
  generatedMessage: true,
  code: 'ERR_ASSERTION',
  actual: false,
  expected: true,
  operator: '=='
}

This is caused by a call to Isolate::GetCurrent() when the calling
thread has not initialized V8. We are working suggestion to add a method
to V8 which allows a check/get without any checks but in the mean time
this change should allow debug builds to pass the test suit.

PR-URL: https://github.com/nodejs/node/pull/38805
Refs: https://chromium-review.googlesource.com/c/v8/v8/+/2910630
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
2021-06-08 07:57:01 +02:00
legendecas
f536cc2e89
src: fix fatal errors when a current isolate not exist
napi_fatal_error and node watchdog trigger fatal error but rather
running on a thread that hold no current isolate.

PR-URL: https://github.com/nodejs/node/pull/38624
Reviewed-By: Michael Dawson <midawson@redhat.com>
2021-05-18 09:44:59 +08:00
Fedor Indutny
7abc7e45b2 node-api: faster threadsafe_function
Invoke threadsafe_function during the same tick and avoid marshalling
costs between threads and/or churning event loop if either:

1. There's a queued call already
2. `Push()` is called while the main thread was running
   threadsafe_function

PR-URL: https://github.com/nodejs/node/pull/38506
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
2021-05-04 18:39:28 -07:00
Tobias Nießen
8f4850d5c7 test: fix typo in comment in binding.c
PR-URL: https://github.com/nodejs/node/pull/38220
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Beth Griggs <bgriggs@redhat.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
2021-04-13 18:57:17 +05:30
Rich Trott
330f25ef82 test: prepare for consistent comma-dangle lint rule
Make changes so that tests will pass when the comma-dangle settings
applied to the rest of the code base are also applied to tests.

PR-URL: https://github.com/nodejs/node/pull/37930
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Derek Lewis <DerekNonGeneric@inf.is>
2021-04-01 23:14:29 -07:00
Gabriel Schulhof
52f9aafeab node-api: stop ref gc during environment teardown
A gc may happen during environment teardown. Thus, during finalization
initiated by environment teardown we must remove the V8 finalizer
before calling the Node-API finalizer.

Fixes: https://github.com/nodejs/node/issues/37236
PR-URL: https://github.com/nodejs/node/pull/37616
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
2021-03-18 20:40:59 -07:00
Gabriel Schulhof
d15475578a node-api: define version 8
Mark as stable the APIs that define Node-API version 8.

PR-URL: https://github.com/nodejs/node/pull/37652
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
2021-03-10 16:31:16 -05:00
Gabriel Schulhof
ad3ebed046 node-api: allow retrieval of add-on file name
Unlike JS-only modules, native add-ons are always associated with a
dynamic shared object from which they are loaded. Being able to
retrieve its absolute path is important to native-only add-ons, i.e.
add-ons that are not themselves being loaded from a JS-only module
located in the same package as the native add-on itself.

Currently, the file name is obtained at environment construction time
from the JS `module.filename`. Nevertheless, the presence of `module`
is not required, because the file name could also be passed in via a
private property added onto `exports` from the `process.dlopen`
binding.

As an attempt at future-proofing, the file name is provided as a URL,
i.e. prefixed with the `file://` protocol.

Fixes: https://github.com/nodejs/node-addon-api/issues/449
PR-URL: https://github.com/nodejs/node/pull/37195
Co-authored-by: Michael Dawson <mdawson@devrus.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
2021-02-09 23:34:30 -08:00
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
Michaël Zasso
501ae0e6e3
test: remove outdated V8 flag
The flag is going to be removed upstream in V8 9.0.

Refs: 0a480c30d5

PR-URL: https://github.com/nodejs/node/pull/37151
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
2021-02-02 10:33:56 +01:00
Rich Trott
996b85b5c2 test,doc,lib: adjust object literal newlines for lint rule
Before enabling object-curly-newline for our ESLint rules, adjust files
to comply with it.

Refs: https://eslint.org/docs/rules/object-curly-newline

PR-URL: https://github.com/nodejs/node/pull/37040
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
2021-01-26 16:49:18 -08:00
Gabriel Schulhof
551d8c4b71 n-api: fix test_async_context warnings
Signed-off-by: Gabriel Schulhof <gabriel.schulhof@intel.com>
PR-URL: https://github.com/nodejs/node/pull/36171
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
2020-11-20 13:42:35 -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
legendecas
3a7537de7d n-api: napi_make_callback emit async init with resource of async_context
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 <admin@stephenbelanger.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
2020-10-31 00:17:59 +00: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
Rich Trott
f593f9e9ef test: adjust comments for upcoming lint rule
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 <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
2020-10-07 04:36:02 -07:00
Gabriel Schulhof
0848f56cb3 n-api: re-implement async env cleanup hooks
* 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 <gabriel.schulhof@intel.com>
Co-authored-by: Anna Henningsen <github@addaleax.net>
PR-URL: https://github.com/nodejs/node/pull/34819
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Zeyu Yang <himself65@outlook.com>
2020-08-27 08:18:37 -07: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
Anna Henningsen
22cbbcf9d9
n-api,src: provide asynchronous cleanup hooks
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 <jasnell@gmail.com>
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
2020-08-07 00:02:02 +02:00
Gabriel Schulhof
734327280d test: convert most N-API tests from C++ to C
* Prefix functions with `static` to make them local
* Remove anonymous namespaces
* `nullptr` -> `NULL`
* .cc -> .c and update binding.gyp
* `static_cast<x>()` -> `(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 <vector>`
  (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 <gabriel.schulhof@intel.com>
PR-URL: https://github.com/nodejs/node/pull/34615
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
2020-08-05 14:21:51 -07: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
Antoine du HAMEL
6443ab9595 module: deprecate module.parent
This feature does not work when a module is imported using ECMAScript
modules specification, therefore it is deprecated.

Fixes: https://github.com/nodejs/modules/issues/469

PR-URL: https://github.com/nodejs/node/pull/32217
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Bradley Farias <bradley.meck@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
2020-05-24 03:38:39 +02:00
Anna Henningsen
b18d8dde84 Revert "n-api: detect deadlocks in thread-safe function"
This reverts commit d26ca06c16 because
it breaks running the tests in debug mode, as
`v8::Isolate::GetCurrent()` is not allowed if no `Isolate` is active
on the current thread.

Refs: https://github.com/nodejs/node/pull/33276
Refs: https://github.com/nodejs/node/pull/32860

PR-URL: https://github.com/nodejs/node/pull/33453
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
2020-05-23 17:45:42 +02:00
Anna Henningsen
c1ee70ec16
buffer,n-api: release external buffers from BackingStore callback
Release `Buffer` and `ArrayBuffer` instances that were created through
our addon APIs and have finalizers attached to them only after V8 has
called the deleter callback passed to the `BackingStore`, instead of
relying on our own GC callback(s).

This fixes the following race condition:

1. Addon code allocates pointer P via `malloc`.
2. P is passed into `napi_create_external_buffer` with a finalization
   callback which calls `free(P)`. P is inserted into V8’s global array
   buffer table for tracking.
3. The finalization callback is executed on GC. P is freed and returned
   to the allocator. P is not yet removed from V8’s global array
   buffer table. (!)
4. Addon code attempts to allocate memory once again. The allocator
   returns P, as it is now available.
5. P is passed into `napi_create_external_buffer`. P still has not been
   removed from the v8 global array buffer table.
6. The world ends with `Check failed: result.second`.

Since our API contract is to call the finalizer on the JS thread on
which the `ArrayBuffer` was created, but V8 may call the `BackingStore`
deleter callback on another thread, fixing this requires posting
a task back to the JS thread.

Refs: https://github.com/nodejs/node/issues/32463#issuecomment-625877175
Fixes: https://github.com/nodejs/node/issues/32463

PR-URL: https://github.com/nodejs/node/pull/33321
Reviewed-By: James M Snell <jasnell@gmail.com>
2020-05-16 12:15:07 +02:00
Anna Henningsen
8698dd98bb
test: fix out-of-bound reads from invalid sizeof usage
`sizeof(data)` does not return the correct result here, as it measures
the size of the `data` variable, not what it points to.

PR-URL: https://github.com/nodejs/node/pull/33115
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
2020-04-30 04:56:11 +02:00
Anna Henningsen
ff38ec0471
test: add missing calls to napi_async_destroy
PR-URL: https://github.com/nodejs/node/pull/33114
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
2020-04-30 04:52:04 +02:00
legendecas
307c67be17
n-api: fix false assumption on napi_async_context structures
napi_async_context should be an opaque type and not be used as same as
node::async_context.

PR-URL: https://github.com/nodejs/node/pull/32928
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
2020-04-28 01:03:05 +02:00
Gabriel Schulhof
d26ca06c16 n-api: detect deadlocks in thread-safe function
We introduce status `napi_would_deadlock` to be used as a return status
by `napi_call_threadsafe_function` if the call is made with
`napi_tsfn_blocking` on the main thread and the queue is full.

Fixes: https://github.com/nodejs/node/issues/32615
Signed-off-by: Gabriel Schulhof <gabriel.schulhof@intel.com>
PR-URL: https://github.com/nodejs/node/pull/32860
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Zeyu Yang <himself65@outlook.com>
2020-04-19 10:07:00 -07:00
Gabriel Schulhof
d3d5eca657 Revert "n-api: detect deadlocks in thread-safe function"
This reverts commit aeb7084fe6.

The solution creates incorrect behaviour on Windows.

Re: https://github.com/nodejs/node-addon-api/pull/697#issuecomment-612993476
Signed-off-by: Gabriel Schulhof <gabriel.schulhof@intel.com>
PR-URL: https://github.com/nodejs/node/pull/32880
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
2020-04-16 12:52:12 -07:00
Gabriel Schulhof
aeb7084fe6 n-api: detect deadlocks in thread-safe function
We introduce status `napi_would_deadlock` to be used as a return status
by `napi_call_threadsafe_function` if the call is made with
`napi_tsfn_blocking` on the main thread and the queue is full.

PR-URL: https://github.com/nodejs/node/pull/32689
Fixes: https://github.com/nodejs/node/issues/32615
Signed-off-by: Gabriel Schulhof <gabriel.schulhof@intel.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
2020-04-09 02:41:28 -07:00
Gabriel Schulhof
c0c81ed204 n-api: define release 6
Mark all N-APIs that have been added since version 5 as stable.

PR-URL: https://github.com/nodejs/node/pull/32058
Fixes: https://github.com/nodejs/abi-stable-node/issues/393
Co-Authored-By: legendecas <legendecas@gmail.com>
Signed-off-by: Gabriel Schulhof <gabriel.schulhof@intel.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
2020-03-09 11:58:51 -07:00
Gabriel Schulhof
884e287199 n-api: free instance data as reference
Instance data associated with a `napi_env` is no longer stored on the
env itself but is instead rendered as a reference. Since
`v8impl::Reference` is tied to a JS object, this modification factors
out the `v8impl::Reference` refcounting and the deletion process into
a base class for `v8impl::Reference`, called `v8impl::RefBase`. The
instance data is then stored as a `v8impl::RefBase`, along with other
references, preventing a segfault that arises from the fact that, up
until now, upon `napi_env` destruction, the instance data was freed
after all references had already been forcefully freed. If the addon
freed a reference during the `napi_set_instance_data` finalizer
callback, such a reference had already been freed during environment
teardown, causing a double free.

Re: https://github.com/nodejs/node-addon-api/pull/663
PR-URL: https://github.com/nodejs/node/pull/31638
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: David Carlier <devnexen@gmail.com>
2020-02-06 12:43:24 -08:00