Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add support for requiring basic finalizers #1568

Merged
merged 2 commits into from
Sep 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
/benchmark/build
/benchmark/src
/test/addon_build/addons
/test/require_basic_finalizers/addons
/.vscode

# ignore node-gyp generated files outside its build directory
Expand Down
4 changes: 3 additions & 1 deletion doc/finalization.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ provide more efficient memory management, optimizations, improved execution, or
other benefits.

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

## Finalizers

Expand Down
21 changes: 16 additions & 5 deletions doc/setup.md
Original file line number Diff line number Diff line change
Expand Up @@ -81,12 +81,23 @@ To use **Node-API** in a native module:
At build time, the Node-API back-compat library code will be used only when the
targeted node version *does not* have Node-API built-in.

The preprocessor directive `NODE_ADDON_API_DISABLE_DEPRECATED` can be defined at
compile time before including `napi.h` to skip the definition of deprecated APIs.
The `NODE_ADDON_API_DISABLE_DEPRECATED` preprocessor directive can be defined at
compile time before including `napi.h` to skip the definition of deprecated
APIs.

By default, throwing an exception on a terminating environment (eg. worker
threads) will cause a fatal exception, terminating the Node process. This is to
provide feedback to the user of the runtime error, as it is impossible to pass
the error to JavaScript when the environment is terminating. In order to bypass
this behavior such that the Node process will not terminate, define the
preprocessor directive `NODE_API_SWALLOW_UNTHROWABLE_EXCEPTIONS`.
the error to JavaScript when the environment is terminating. The
`NODE_API_SWALLOW_UNTHROWABLE_EXCEPTIONS` preprocessor directive can be defined
to bypass this behavior, such that the Node process will not terminate.

Various Node-API constructs provide a mechanism to run a callback in response to
a garbage collection event of that object. These callbacks are called
[_finalizers_]. Some finalizers have restrictions on the type of Node-APIs
available within the callback. node-addon-api provides convenience helpers that
bypass this limitation, but may cause the add-on to run less efficiently. The
`NODE_ADDON_API_REQUIRE_BASIC_FINALIZERS` preprocessor directive can be defined
to disable the convenience helpers.

[_finalizers_]: ./finalization.md
10 changes: 10 additions & 0 deletions napi-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,11 @@ struct FinalizeData {
static inline void Wrapper(node_api_nogc_env env,
void* data,
void* finalizeHint) NAPI_NOEXCEPT {
#ifdef NODE_ADDON_API_REQUIRE_BASIC_FINALIZERS
static_assert(false,
"NODE_ADDON_API_REQUIRE_BASIC_FINALIZERS defined: Finalizer "
"must be basic.");
#endif
napi_status status =
node_api_post_finalizer(env, WrapperGC, data, finalizeHint);
NAPI_FATAL_IF_FAILED(
Expand Down Expand Up @@ -243,6 +248,11 @@ struct FinalizeData {
static inline void WrapperWithHint(node_api_nogc_env env,
void* data,
void* finalizeHint) NAPI_NOEXCEPT {
#ifdef NODE_ADDON_API_REQUIRE_BASIC_FINALIZERS
static_assert(false,
"NODE_ADDON_API_REQUIRE_BASIC_FINALIZERS defined: Finalizer "
"must be basic.");
#endif
napi_status status =
node_api_post_finalizer(env, WrapperGCWithHint, data, finalizeHint);
NAPI_FATAL_IF_FAILED(
Expand Down
38 changes: 38 additions & 0 deletions test/require_basic_finalizers/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
'use strict';

const { promisify } = require('util');
const exec = promisify(require('child_process').exec);
const { copy, remove } = require('fs-extra');
const path = require('path');
const assert = require('assert');

async function test () {
const addon = 'require-basic-finalizers';
const ADDON_FOLDER = path.join(__dirname, 'addons', addon);

await remove(ADDON_FOLDER);
await copy(path.join(__dirname, 'tpl'), ADDON_FOLDER);

console.log(' >Building addon');

// Fail when NODE_ADDON_API_REQUIRE_BASIC_FINALIZERS is enabled
await assert.rejects(exec('npm --require-basic-finalizers install', {
cwd: ADDON_FOLDER
}), 'Addon unexpectedly compiled successfully');

// Succeed when NODE_ADDON_API_REQUIRE_BASIC_FINALIZERS is not enabled
return assert.doesNotReject(exec('npm install', {
cwd: ADDON_FOLDER
}));
}

module.exports = (function () {
// This test will only run under an experimental version test.
const isExperimental = Number(process.env.NAPI_VERSION) === 2147483647;

if (isExperimental) {
return test();
} else {
console.log(' >Skipped (non-experimental test run)');
}
})();
1 change: 1 addition & 0 deletions test/require_basic_finalizers/tpl/.npmrc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
package-lock=false
12 changes: 12 additions & 0 deletions test/require_basic_finalizers/tpl/addon.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
#include <napi.h>

Napi::Object Init(Napi::Env env, Napi::Object exports) {
exports.Set(
"external",
Napi::External<int>::New(
env, new int(1), [](Napi::Env /*env*/, int* data) { delete data; }));

return exports;
}

NODE_API_MODULE(NODE_GYP_MODULE_NAME, Init)
48 changes: 48 additions & 0 deletions test/require_basic_finalizers/tpl/binding.gyp
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
{
'target_defaults': {
'include_dirs': [
"<!(node -p \"require('node-addon-api').include_dir\")"
],
'variables': {
'NAPI_VERSION%': "<!(node -p \"process.env.NAPI_VERSION || process.versions.napi\")",
'require_basic_finalizers': "<!(node -p \"process.env['npm_config_require_basic_finalizers']\")",
},
'conditions': [
['NAPI_VERSION!=""', { 'defines': ['NAPI_VERSION=<@(NAPI_VERSION)'] } ],
['NAPI_VERSION==2147483647', { 'defines': ['NAPI_EXPERIMENTAL'] } ],
['require_basic_finalizers=="true"', {
'defines': ['NODE_ADDON_API_REQUIRE_BASIC_FINALIZERS'],
}],
['OS=="mac"', {
'cflags+': ['-fvisibility=hidden'],
'xcode_settings': {
'OTHER_CFLAGS': ['-fvisibility=hidden']
}
}]
],
'sources': [
'addon.cc',
],
},
'targets': [
{
'target_name': 'addon',
'defines': [
'NAPI_CPP_EXCEPTIONS'
],
'cflags!': [ '-fno-exceptions' ],
'cflags_cc!': [ '-fno-exceptions' ],
'msvs_settings': {
'VCCLCompilerTool': {
'ExceptionHandling': 1,
'EnablePREfast': 'true',
},
},
'xcode_settings': {
'CLANG_CXX_LIBRARY': 'libc++',
'MACOSX_DEPLOYMENT_TARGET': '10.7',
'GCC_ENABLE_CPP_EXCEPTIONS': 'YES',
},
}
]
}
3 changes: 3 additions & 0 deletions test/require_basic_finalizers/tpl/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
'use strict';

module.exports = require('bindings')('addon');
11 changes: 11 additions & 0 deletions test/require_basic_finalizers/tpl/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"name": "addon",
"version": "0.0.0",
"description": "Node.js addon",
"private": true,
"scripts": {},
"dependencies": {
"node-addon-api": "file:../../../../"
},
"gypfile": true
}