diff --git a/.gitignore b/.gitignore index f6b80ae92..a154db646 100644 --- a/.gitignore +++ b/.gitignore @@ -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 diff --git a/doc/finalization.md b/doc/finalization.md index 825ff742a..3dc4e7860 100644 --- a/doc/finalization.md +++ b/doc/finalization.md @@ -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 diff --git a/doc/setup.md b/doc/setup.md index 29bb1a743..86a1b0f38 100644 --- a/doc/setup.md +++ b/doc/setup.md @@ -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 diff --git a/napi-inl.h b/napi-inl.h index c0f711b68..e7e21a7b3 100644 --- a/napi-inl.h +++ b/napi-inl.h @@ -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( @@ -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( diff --git a/test/require_basic_finalizers/index.js b/test/require_basic_finalizers/index.js new file mode 100644 index 000000000..31ee4f00b --- /dev/null +++ b/test/require_basic_finalizers/index.js @@ -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)'); + } +})(); diff --git a/test/require_basic_finalizers/tpl/.npmrc b/test/require_basic_finalizers/tpl/.npmrc new file mode 100644 index 000000000..43c97e719 --- /dev/null +++ b/test/require_basic_finalizers/tpl/.npmrc @@ -0,0 +1 @@ +package-lock=false diff --git a/test/require_basic_finalizers/tpl/addon.cc b/test/require_basic_finalizers/tpl/addon.cc new file mode 100644 index 000000000..f4277ac74 --- /dev/null +++ b/test/require_basic_finalizers/tpl/addon.cc @@ -0,0 +1,12 @@ +#include + +Napi::Object Init(Napi::Env env, Napi::Object exports) { + exports.Set( + "external", + Napi::External::New( + env, new int(1), [](Napi::Env /*env*/, int* data) { delete data; })); + + return exports; +} + +NODE_API_MODULE(NODE_GYP_MODULE_NAME, Init) diff --git a/test/require_basic_finalizers/tpl/binding.gyp b/test/require_basic_finalizers/tpl/binding.gyp new file mode 100644 index 000000000..caf99d21f --- /dev/null +++ b/test/require_basic_finalizers/tpl/binding.gyp @@ -0,0 +1,48 @@ +{ + 'target_defaults': { + 'include_dirs': [ + "