Skip to content

Commit 7c91a10

Browse files
KevinEadyKevinEady
committed
feat: add support for requiring basic finalizers
1 parent b4aeecb commit 7c91a10

File tree

10 files changed

+139
-3
lines changed

10 files changed

+139
-3
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). To ensure that all finalizers are basic
12+
finalizers at compile-time, define the `NODE_ADDON_API_REQUIRE_BASIC_FINALIZERS`
13+
preprocessor directive.
1214

1315
## Finalizers
1416

‎doc/setup.md‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,3 +90,13 @@ provide feedback to the user of the runtime error, as it is impossible to pass
9090
the error to JavaScript when the environment is terminating. In order to bypass
9191
this behavior such that the Node process will not terminate, define the
9292
preprocessor directive `NODE_API_SWALLOW_UNTHROWABLE_EXCEPTIONS`.
93+
94+
Various Node-API constructs provide a mechanism to run a callback in response to
95+
a garbage collection event of that object. These callbacks are called
96+
[_finalizers_]. Some finalizers have restrictions on the type of Node-APIs
97+
available within the callback. node-addon-api provides convenience helpers that
98+
bypass this limitation, but may cause the add-on to run less efficiently. To
99+
disable the convenience helpers, define the preprocessor directive
100+
`NODE_ADDON_API_REQUIRE_BASIC_FINALIZERS`.
101+
102+
[_finalizers_]: ./finalization.md

‎napi-inl.h‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -205,14 +205,19 @@ struct FinalizeData {
205205
});
206206
}
207207

208-
#ifdef NODE_API_EXPERIMENTAL_HAS_POST_FINALIZER
208+
#if defined(NODE_API_EXPERIMENTAL_HAS_POST_FINALIZER)
209209
template <typename F = Finalizer,
210210
typename = std::enable_if_t<
211211
!std::is_invocable_v<F, node_api_nogc_env, T*>>,
212212
typename = void>
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(
@@ -235,14 +240,19 @@ struct FinalizeData {
235240
});
236241
}
237242

238-
#ifdef NODE_API_EXPERIMENTAL_HAS_POST_FINALIZER
243+
#if defined(NODE_API_EXPERIMENTAL_HAS_POST_FINALIZER)
239244
template <typename F = Finalizer,
240245
typename = std::enable_if_t<
241246
!std::is_invocable_v<F, node_api_nogc_env, T*, Hint*>>,
242247
typename = void>
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)