Skip to content

Commit 8f78a76

Browse files
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 8192a47 commit 8f78a76

File tree

3 files changed

+61
-7
lines changed

3 files changed

+61
-7
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: [ 16.x, 18.x, 20.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

+54-7
Original file line numberDiff line numberDiff line change
@@ -180,11 +180,36 @@ napi_value TemplatedInstanceVoidCallback(napi_env env, napi_callback_info info)
180180
});
181181
}
182182

183+
inline void DeferredCallback(const char* placeOfFailure,
184+
napi_env env,
185+
void* data,
186+
void* hint,
187+
napi_finalize fini) {
188+
#ifdef NAPI_EXPERIMENTAL
189+
napi_status status = node_api_post_finalizer(env, fini, data, hint);
190+
NAPI_FATAL_IF_FAILED(
191+
status, placeOfFailure, "DeferredCallback -> node_api_post_finalizer");
192+
#else
193+
(void)placeOfFailure;
194+
fini(env, data, hint);
195+
#endif
196+
}
197+
183198
template <typename T, typename Finalizer, typename Hint = void>
184199
struct FinalizeData {
185200
static inline void Wrapper(napi_env env,
186201
void* data,
187202
void* finalizeHint) NAPI_NOEXCEPT {
203+
DeferredCallback("FinalizeData::Wrapper",
204+
env,
205+
data,
206+
finalizeHint,
207+
FinalizeData<T, Finalizer>::SyncWrapper);
208+
}
209+
210+
static inline void SyncWrapper(napi_env env,
211+
void* data,
212+
void* finalizeHint) NAPI_NOEXCEPT {
188213
WrapVoidCallback([&] {
189214
FinalizeData* finalizeData = static_cast<FinalizeData*>(finalizeHint);
190215
finalizeData->callback(Env(env), static_cast<T*>(data));
@@ -195,6 +220,16 @@ struct FinalizeData {
195220
static inline void WrapperWithHint(napi_env env,
196221
void* data,
197222
void* finalizeHint) NAPI_NOEXCEPT {
223+
DeferredCallback("FinalizeData::WrapperWithHint",
224+
env,
225+
data,
226+
finalizeHint,
227+
FinalizeData<T, Finalizer, Hint>::SyncWrapperWithHint);
228+
}
229+
230+
static inline void SyncWrapperWithHint(napi_env env,
231+
void* data,
232+
void* finalizeHint) NAPI_NOEXCEPT {
198233
WrapVoidCallback([&] {
199234
FinalizeData* finalizeData = static_cast<FinalizeData*>(finalizeHint);
200235
finalizeData->callback(
@@ -2731,7 +2766,7 @@ inline Buffer<T> Buffer<T>::NewOrCopy(napi_env env,
27312766
#endif // NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED
27322767
// If we can't create an external buffer, we'll just copy the data.
27332768
Buffer<T> ret = Buffer<T>::Copy(env, data, length);
2734-
details::FinalizeData<T, Finalizer>::Wrapper(env, data, finalizeData);
2769+
details::FinalizeData<T, Finalizer>::SyncWrapper(env, data, finalizeData);
27352770
return ret;
27362771
#ifndef NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED
27372772
}
@@ -2766,7 +2801,7 @@ inline Buffer<T> Buffer<T>::NewOrCopy(napi_env env,
27662801
#endif
27672802
// If we can't create an external buffer, we'll just copy the data.
27682803
Buffer<T> ret = Buffer<T>::Copy(env, data, length);
2769-
details::FinalizeData<T, Finalizer, Hint>::WrapperWithHint(
2804+
details::FinalizeData<T, Finalizer, Hint>::SyncWrapperWithHint(
27702805
env, data, finalizeData);
27712806
return ret;
27722807
#ifndef NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED
@@ -3054,7 +3089,13 @@ inline void Error::ThrowAsJavaScriptException() const {
30543089

30553090
status = napi_throw(_env, Value());
30563091

3057-
if (status == napi_pending_exception) {
3092+
#ifdef NAPI_EXPERIMENTAL
3093+
napi_status expected_failure_mode = napi_cannot_run_js;
3094+
#else
3095+
napi_status expected_failure_mode = napi_pending_exception;
3096+
#endif
3097+
3098+
if (status == expected_failure_mode) {
30583099
// The environment must be terminating as we checked earlier and there
30593100
// was no pending exception. In this case continuing will result
30603101
// in a fatal error and there is nothing the author has done incorrectly
@@ -4879,10 +4920,16 @@ template <typename T>
48794920
inline void ObjectWrap<T>::FinalizeCallback(napi_env env,
48804921
void* data,
48814922
void* /*hint*/) {
4882-
HandleScope scope(env);
4883-
T* instance = static_cast<T*>(data);
4884-
instance->Finalize(Napi::Env(env));
4885-
delete instance;
4923+
details::DeferredCallback("ObjectWrap<T>::FinalizeCallback",
4924+
env,
4925+
data,
4926+
nullptr,
4927+
[](napi_env env, void* data, void* /*hint*/) {
4928+
HandleScope scope(env);
4929+
T* instance = static_cast<T*>(data);
4930+
instance->Finalize(Napi::Env(env));
4931+
delete instance;
4932+
});
48864933
}
48874934

48884935
template <typename T>

0 commit comments

Comments
 (0)