Skip to content

Commit 7bcb826

Browse files
KevinEadyKevinEady
authored
feat: add support for requiring basic finalizers (#1568)
* feat: add support for requiring basic finalizers * Address review comments - Use passive voice in existing and new docs - Revert unnecessary change
1 parent 294a43f commit 7bcb826

File tree

10 files changed

+143
-6
lines changed

10 files changed

+143
-6
lines changed

‎.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
/benchmark/build
44
/benchmark/src
55
/test/addon_build/addons
6+
/test/require_basic_finalizers/addons
67
/.vscode
78

89
# ignore node-gyp generated files outside its build directory

‎doc/finalization.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,9 @@ provide more efficient memory management, optimizations, improved execution, or
88
other benefits.
99

1010
In general, it is best to use basic finalizers whenever possible (eg. when
11-
access to JavaScript is _not_ needed).
11+
access to JavaScript is _not_ needed). The
12+
`NODE_ADDON_API_REQUIRE_BASIC_FINALIZERS` preprocessor directive can be defined
13+
to ensure that all finalizers are basic.
1214

1315
## Finalizers
1416

‎doc/setup.md

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -81,12 +81,23 @@ To use **Node-API** in a native module:
8181
At build time, the Node-API back-compat library code will be used only when the
8282
targeted node version *does not* have Node-API built-in.
8383

84-
The preprocessor directive `NODE_ADDON_API_DISABLE_DEPRECATED` can be defined at
85-
compile time before including `napi.h` to skip the definition of deprecated APIs.
84+
The `NODE_ADDON_API_DISABLE_DEPRECATED` preprocessor directive can be defined at
85+
compile time before including `napi.h` to skip the definition of deprecated
86+
APIs.
8687

8788
By default, throwing an exception on a terminating environment (eg. worker
8889
threads) will cause a fatal exception, terminating the Node process. This is to
8990
provide feedback to the user of the runtime error, as it is impossible to pass
90-
the error to JavaScript when the environment is terminating. In order to bypass
91-
this behavior such that the Node process will not terminate, define the
92-
preprocessor directive `NODE_API_SWALLOW_UNTHROWABLE_EXCEPTIONS`.
91+
the error to JavaScript when the environment is terminating. The
92+
`NODE_API_SWALLOW_UNTHROWABLE_EXCEPTIONS` preprocessor directive can be defined
93+
to bypass this behavior, such that the Node process will not terminate.
94+
95+
Various Node-API constructs provide a mechanism to run a callback in response to
96+
a garbage collection event of that object. These callbacks are called
97+
[_finalizers_]. Some finalizers have restrictions on the type of Node-APIs
98+
available within the callback. node-addon-api provides convenience helpers that
99+
bypass this limitation, but may cause the add-on to run less efficiently. The
100+
`NODE_ADDON_API_REQUIRE_BASIC_FINALIZERS` preprocessor directive can be defined
101+
to disable the convenience helpers.
102+
103+
[_finalizers_]: ./finalization.md

‎napi-inl.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,11 @@ struct FinalizeData {
213213
static inline void Wrapper(node_api_nogc_env env,
214214
void* data,
215215
void* finalizeHint) NAPI_NOEXCEPT {
216+
#ifdef NODE_ADDON_API_REQUIRE_BASIC_FINALIZERS
217+
static_assert(false,
218+
"NODE_ADDON_API_REQUIRE_BASIC_FINALIZERS defined: Finalizer "
219+
"must be basic.");
220+
#endif
216221
napi_status status =
217222
node_api_post_finalizer(env, WrapperGC, data, finalizeHint);
218223
NAPI_FATAL_IF_FAILED(
@@ -243,6 +248,11 @@ struct FinalizeData {
243248
static inline void WrapperWithHint(node_api_nogc_env env,
244249
void* data,
245250
void* finalizeHint) NAPI_NOEXCEPT {
251+
#ifdef NODE_ADDON_API_REQUIRE_BASIC_FINALIZERS
252+
static_assert(false,
253+
"NODE_ADDON_API_REQUIRE_BASIC_FINALIZERS defined: Finalizer "
254+
"must be basic.");
255+
#endif
246256
napi_status status =
247257
node_api_post_finalizer(env, WrapperGCWithHint, data, finalizeHint);
248258
NAPI_FATAL_IF_FAILED(

‎test/require_basic_finalizers/index.js

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
'use strict';
2+
3+
const { promisify } = require('util');
4+
const exec = promisify(require('child_process').exec);
5+
const { copy, remove } = require('fs-extra');
6+
const path = require('path');
7+
const assert = require('assert');
8+
9+
async function test () {
10+
const addon = 'require-basic-finalizers';
11+
const ADDON_FOLDER = path.join(__dirname, 'addons', addon);
12+
13+
await remove(ADDON_FOLDER);
14+
await copy(path.join(__dirname, 'tpl'), ADDON_FOLDER);
15+
16+
console.log(' >Building addon');
17+
18+
// Fail when NODE_ADDON_API_REQUIRE_BASIC_FINALIZERS is enabled
19+
await assert.rejects(exec('npm --require-basic-finalizers install', {
20+
cwd: ADDON_FOLDER
21+
}), 'Addon unexpectedly compiled successfully');
22+
23+
// Succeed when NODE_ADDON_API_REQUIRE_BASIC_FINALIZERS is not enabled
24+
return assert.doesNotReject(exec('npm install', {
25+
cwd: ADDON_FOLDER
26+
}));
27+
}
28+
29+
module.exports = (function () {
30+
// This test will only run under an experimental version test.
31+
const isExperimental = Number(process.env.NAPI_VERSION) === 2147483647;
32+
33+
if (isExperimental) {
34+
return test();
35+
} else {
36+
console.log(' >Skipped (non-experimental test run)');
37+
}
38+
})();

‎test/require_basic_finalizers/tpl/.npmrc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
package-lock=false

‎test/require_basic_finalizers/tpl/addon.cc

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
#include <napi.h>
2+
3+
Napi::Object Init(Napi::Env env, Napi::Object exports) {
4+
exports.Set(
5+
"external",
6+
Napi::External<int>::New(
7+
env, new int(1), [](Napi::Env /*env*/, int* data) { delete data; }));
8+
9+
return exports;
10+
}
11+
12+
NODE_API_MODULE(NODE_GYP_MODULE_NAME, Init)

‎test/require_basic_finalizers/tpl/binding.gyp

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
{
2+
'target_defaults': {
3+
'include_dirs': [
4+
"<!(node -p \"require('node-addon-api').include_dir\")"
5+
],
6+
'variables': {
7+
'NAPI_VERSION%': "<!(node -p \"process.env.NAPI_VERSION || process.versions.napi\")",
8+
'require_basic_finalizers': "<!(node -p \"process.env['npm_config_require_basic_finalizers']\")",
9+
},
10+
'conditions': [
11+
['NAPI_VERSION!=""', { 'defines': ['NAPI_VERSION=<@(NAPI_VERSION)'] } ],
12+
['NAPI_VERSION==2147483647', { 'defines': ['NAPI_EXPERIMENTAL'] } ],
13+
['require_basic_finalizers=="true"', {
14+
'defines': ['NODE_ADDON_API_REQUIRE_BASIC_FINALIZERS'],
15+
}],
16+
['OS=="mac"', {
17+
'cflags+': ['-fvisibility=hidden'],
18+
'xcode_settings': {
19+
'OTHER_CFLAGS': ['-fvisibility=hidden']
20+
}
21+
}]
22+
],
23+
'sources': [
24+
'addon.cc',
25+
],
26+
},
27+
'targets': [
28+
{
29+
'target_name': 'addon',
30+
'defines': [
31+
'NAPI_CPP_EXCEPTIONS'
32+
],
33+
'cflags!': [ '-fno-exceptions' ],
34+
'cflags_cc!': [ '-fno-exceptions' ],
35+
'msvs_settings': {
36+
'VCCLCompilerTool': {
37+
'ExceptionHandling': 1,
38+
'EnablePREfast': 'true',
39+
},
40+
},
41+
'xcode_settings': {
42+
'CLANG_CXX_LIBRARY': 'libc++',
43+
'MACOSX_DEPLOYMENT_TARGET': '10.7',
44+
'GCC_ENABLE_CPP_EXCEPTIONS': 'YES',
45+
},
46+
}
47+
]
48+
}

‎test/require_basic_finalizers/tpl/index.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
'use strict';
2+
3+
module.exports = require('bindings')('addon');

‎test/require_basic_finalizers/tpl/package.json

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
{
2+
"name": "addon",
3+
"version": "0.0.0",
4+
"description": "Node.js addon",
5+
"private": true,
6+
"scripts": {},
7+
"dependencies": {
8+
"node-addon-api": "file:../../../../"
9+
},
10+
"gypfile": true
11+
}

0 commit comments

Comments
 (0)