Skip to content

Commit 2c5853a

Browse files
gabrielschulhofgabrielschulhof
committed
src: restore ability to run under NAPI_EXPERIMENTAL
Since we made the default for Node.js core finalizers synchronous for users running with `NAPI_EXPERIMENTAL` and introduced `env->CheckGCAccess()` in Node.js core, we must now defer all finalizers in node-addon-api, because our users likely make non-gc-safe Node-API calls from existing finalizers. To that end, * Use the NAPI_VERSION environment variable to detect whether `NAPI_EXPERIMENTAL` should be on, and add it to the defines if `NAPI_VERSION` is set to `NAPI_VERSION_EXPERIMENTAL`, i.e. 2147483647. * When building with `NAPI_EXPERIMENTAL`, * render all finalizers asynchronous, and * expect `napi_cannot_run_js` instead of `napi_exception_pending`.
1 parent f85e814 commit 2c5853a

File tree

3 files changed

+66
-23
lines changed

3 files changed

+66
-23
lines changed

‎.github/workflows/ci.yml

+6
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,9 @@ jobs:
1313
timeout-minutes: 30
1414
strategy:
1515
matrix:
16+
api_version:
17+
- standard
18+
- experimental
1619
node-version: [ 18.x, 20.x, 21.x ]
1720
os:
1821
- macos-latest
@@ -43,6 +46,9 @@ jobs:
4346
npm install
4447
- name: npm test
4548
run: |
49+
if [ "${{ matrix.api_version }}" = "experimental" ]; then
50+
export NAPI_VERSION=2147483647
51+
fi
4652
if [ "${{ matrix.compiler }}" = "gcc" ]; then
4753
export CC="gcc" CXX="g++"
4854
fi

‎common.gypi

+1
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
},
66
'conditions': [
77
['NAPI_VERSION!=""', { 'defines': ['NAPI_VERSION=<@(NAPI_VERSION)'] } ],
8+
['NAPI_VERSION==2147483647', { 'defines': ['NAPI_EXPERIMENTAL'] } ],
89
['disable_deprecated=="true"', {
910
'defines': ['NODE_ADDON_API_DISABLE_DEPRECATED']
1011
}],

‎napi-inl.h

+59-23
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,23 @@ namespace details {
3131
// Node.js releases. Only necessary when they are used in napi.h and napi-inl.h.
3232
constexpr int napi_no_external_buffers_allowed = 22;
3333

34+
#if (defined(NAPI_EXPERIMENTAL) && \
35+
defined(NODE_API_EXPERIMENTAL_HAS_POST_FINALIZER))
36+
template <napi_finalize finalizer>
37+
inline void PostFinalizerWrapper(node_api_nogc_env nogc_env,
38+
void* data,
39+
void* hint) {
40+
napi_status status = node_api_post_finalizer(nogc_env, finalizer, data, hint);
41+
NAPI_FATAL_IF_FAILED(
42+
status, "PostFinalizerWrapper", "node_api_post_finalizer failed");
43+
}
44+
#else
45+
template <napi_finalize finalizer>
46+
inline void PostFinalizerWrapper(napi_env env, void* data, void* hint) {
47+
finalizer(env, data, hint);
48+
}
49+
#endif
50+
3451
template <typename FreeType>
3552
inline void default_finalizer(napi_env /*env*/, void* data, void* /*hint*/) {
3653
delete static_cast<FreeType*>(data);
@@ -65,7 +82,8 @@ inline napi_status AttachData(napi_env env,
6582
}
6683
}
6784
#else // NAPI_VERSION >= 5
68-
status = napi_add_finalizer(env, obj, data, finalizer, hint, nullptr);
85+
status = napi_add_finalizer(
86+
env, obj, data, details::PostFinalizerWrapper<finalizer>, hint, nullptr);
6987
#endif
7088
return status;
7189
}
@@ -1774,7 +1792,8 @@ inline External<T> External<T>::New(napi_env env,
17741792
napi_status status =
17751793
napi_create_external(env,
17761794
data,
1777-
details::FinalizeData<T, Finalizer>::Wrapper,
1795+
details::PostFinalizerWrapper<
1796+
details::FinalizeData<T, Finalizer>::Wrapper>,
17781797
finalizeData,
17791798
&value);
17801799
if (status != napi_ok) {
@@ -1797,7 +1816,8 @@ inline External<T> External<T>::New(napi_env env,
17971816
napi_status status = napi_create_external(
17981817
env,
17991818
data,
1800-
details::FinalizeData<T, Finalizer, Hint>::WrapperWithHint,
1819+
details::PostFinalizerWrapper<
1820+
details::FinalizeData<T, Finalizer, Hint>::WrapperWithHint>,
18011821
finalizeData,
18021822
&value);
18031823
if (status != napi_ok) {
@@ -1910,7 +1930,8 @@ inline ArrayBuffer ArrayBuffer::New(napi_env env,
19101930
env,
19111931
externalData,
19121932
byteLength,
1913-
details::FinalizeData<void, Finalizer>::Wrapper,
1933+
details::PostFinalizerWrapper<
1934+
details::FinalizeData<void, Finalizer>::Wrapper>,
19141935
finalizeData,
19151936
&value);
19161937
if (status != napi_ok) {
@@ -1935,7 +1956,8 @@ inline ArrayBuffer ArrayBuffer::New(napi_env env,
19351956
env,
19361957
externalData,
19371958
byteLength,
1938-
details::FinalizeData<void, Finalizer, Hint>::WrapperWithHint,
1959+
details::PostFinalizerWrapper<
1960+
details::FinalizeData<void, Finalizer, Hint>::WrapperWithHint>,
19391961
finalizeData,
19401962
&value);
19411963
if (status != napi_ok) {
@@ -2652,13 +2674,14 @@ inline Buffer<T> Buffer<T>::New(napi_env env,
26522674
details::FinalizeData<T, Finalizer>* finalizeData =
26532675
new details::FinalizeData<T, Finalizer>(
26542676
{std::move(finalizeCallback), nullptr});
2655-
napi_status status =
2656-
napi_create_external_buffer(env,
2657-
length * sizeof(T),
2658-
data,
2659-
details::FinalizeData<T, Finalizer>::Wrapper,
2660-
finalizeData,
2661-
&value);
2677+
napi_status status = napi_create_external_buffer(
2678+
env,
2679+
length * sizeof(T),
2680+
data,
2681+
details::PostFinalizerWrapper<
2682+
details::FinalizeData<T, Finalizer>::Wrapper>,
2683+
finalizeData,
2684+
&value);
26622685
if (status != napi_ok) {
26632686
delete finalizeData;
26642687
NAPI_THROW_IF_FAILED(env, status, Buffer());
@@ -2681,7 +2704,8 @@ inline Buffer<T> Buffer<T>::New(napi_env env,
26812704
env,
26822705
length * sizeof(T),
26832706
data,
2684-
details::FinalizeData<T, Finalizer, Hint>::WrapperWithHint,
2707+
details::PostFinalizerWrapper<
2708+
details::FinalizeData<T, Finalizer, Hint>::WrapperWithHint>,
26852709
finalizeData,
26862710
&value);
26872711
if (status != napi_ok) {
@@ -2720,13 +2744,14 @@ inline Buffer<T> Buffer<T>::NewOrCopy(napi_env env,
27202744
{std::move(finalizeCallback), nullptr});
27212745
#ifndef NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED
27222746
napi_value value;
2723-
napi_status status =
2724-
napi_create_external_buffer(env,
2725-
length * sizeof(T),
2726-
data,
2727-
details::FinalizeData<T, Finalizer>::Wrapper,
2728-
finalizeData,
2729-
&value);
2747+
napi_status status = napi_create_external_buffer(
2748+
env,
2749+
length * sizeof(T),
2750+
data,
2751+
details::PostFinalizerWrapper<
2752+
details::FinalizeData<T, Finalizer>::Wrapper>,
2753+
finalizeData,
2754+
&value);
27302755
if (status == details::napi_no_external_buffers_allowed) {
27312756
#endif // NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED
27322757
// If we can't create an external buffer, we'll just copy the data.
@@ -2759,7 +2784,8 @@ inline Buffer<T> Buffer<T>::NewOrCopy(napi_env env,
27592784
env,
27602785
length * sizeof(T),
27612786
data,
2762-
details::FinalizeData<T, Finalizer, Hint>::WrapperWithHint,
2787+
details::PostFinalizerWrapper<
2788+
details::FinalizeData<T, Finalizer, Hint>::WrapperWithHint>,
27632789
finalizeData,
27642790
&value);
27652791
if (status == details::napi_no_external_buffers_allowed) {
@@ -3054,7 +3080,12 @@ inline void Error::ThrowAsJavaScriptException() const {
30543080

30553081
status = napi_throw(_env, Value());
30563082

3057-
if (status == napi_pending_exception) {
3083+
#ifdef NAPI_EXPERIMENTAL
3084+
napi_status expected_failure_mode = napi_cannot_run_js;
3085+
#else
3086+
napi_status expected_failure_mode = napi_pending_exception;
3087+
#endif
3088+
if (status == expected_failure_mode) {
30583089
// The environment must be terminating as we checked earlier and there
30593090
// was no pending exception. In this case continuing will result
30603091
// in a fatal error and there is nothing the author has done incorrectly
@@ -4428,7 +4459,12 @@ inline ObjectWrap<T>::ObjectWrap(const Napi::CallbackInfo& callbackInfo) {
44284459
napi_status status;
44294460
napi_ref ref;
44304461
T* instance = static_cast<T*>(this);
4431-
status = napi_wrap(env, wrapper, instance, FinalizeCallback, nullptr, &ref);
4462+
status = napi_wrap(env,
4463+
wrapper,
4464+
instance,
4465+
details::PostFinalizerWrapper<FinalizeCallback>,
4466+
nullptr,
4467+
&ref);
44324468
NAPI_THROW_IF_FAILED_VOID(env, status);
44334469

44344470
Reference<Object>* instanceRef = instance;

0 commit comments

Comments
 (0)