Skip to content

Commit

Permalink
finish renaming async finalizer to basic
Browse files Browse the repository at this point in the history
  • Loading branch information
@KevinEady
KevinEady committed Jul 25, 2024
1 parent 16bca96 commit 5ca9633
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 81 deletions.
40 changes: 19 additions & 21 deletions napi-inl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ namespace details {
constexpr int napi_no_external_buffers_allowed = 22;

template <typename FreeType>
inline void default_finalizer(node_api_nogc_env /*env*/,
void* data,
void* /*hint*/) {
inline void default_basic_finalizer(node_api_nogc_env /*env*/,
void* data,
void* /*hint*/) {
delete static_cast<FreeType*>(data);
}

Expand All @@ -46,7 +46,7 @@ inline void default_finalizer(node_api_nogc_env /*env*/,
// TODO: Replace this code with `napi_add_finalizer()` whenever it becomes
// available on all supported versions of Node.js.
template <typename FreeType,
node_api_nogc_finalize finalizer = default_finalizer<FreeType>>
node_api_nogc_finalize finalizer = default_basic_finalizer<FreeType>>
inline napi_status AttachData(napi_env env,
napi_value obj,
FreeType* data,
Expand Down Expand Up @@ -427,7 +427,7 @@ inline std::string StringFormat(const char* format, ...) {
}

template <typename T>
class HasAsyncFinalizer {
class HasExtendedFinalizer {
private:
template <typename U, void (U::*)(Napi::Env)>
struct SFINAE {};
Expand All @@ -441,7 +441,7 @@ class HasAsyncFinalizer {
};

template <typename T>
class HasSyncFinalizer {
class HasBasicFinalizer {
private:
template <typename U, void (U::*)(Napi::BasicEnv)>
struct SFINAE {};
Expand Down Expand Up @@ -653,22 +653,20 @@ void BasicEnv::CleanupHook<Hook, Arg>::WrapperWithArg(void* data)
#endif // NAPI_VERSION > 2

#if NAPI_VERSION > 5
template <typename T, BasicEnv::AsyncFinalizer<T> async_fini>
template <typename T, BasicEnv::Finalizer<T> fini>
inline void BasicEnv::SetInstanceData(T* data) const {
napi_status status = napi_set_instance_data(
_env,
data,
[](napi_env env, void* data, void*) {
async_fini(env, static_cast<T*>(data));
},
[](napi_env env, void* data, void*) { fini(env, static_cast<T*>(data)); },
nullptr);
NAPI_FATAL_IF_FAILED(
status, "BasicEnv::SetInstanceData", "invalid arguments");
}

template <typename DataType,
typename HintType,
Napi::BasicEnv::AsyncFinalizerWithHint<DataType, HintType> fini>
Napi::BasicEnv::FinalizerWithHint<DataType, HintType> fini>
inline void BasicEnv::SetInstanceData(DataType* data, HintType* hint) const {
napi_status status = napi_set_instance_data(
_env,
Expand All @@ -693,12 +691,12 @@ inline T* BasicEnv::GetInstanceData() const {
}

template <typename T>
void BasicEnv::DefaultAsyncFini(Env, T* data) {
void BasicEnv::DefaultFini(Env, T* data) {
delete data;
}

template <typename DataType, typename HintType>
void BasicEnv::DefaultAsyncFiniWithHint(Env, DataType* data, HintType*) {
void BasicEnv::DefaultFiniWithHint(Env, DataType* data, HintType*) {
delete data;
}
#endif // NAPI_VERSION > 5
Expand Down Expand Up @@ -5018,18 +5016,18 @@ inline void ObjectWrap<T>::FinalizeCallback(node_api_nogc_env env,
// Prevent ~ObjectWrap from calling napi_remove_wrap
instance->_ref = nullptr;

// If class overrides the synchronous finalizer, execute it.
if constexpr (details::HasSyncFinalizer<T>::value) {
// If class overrides the basic finalizer, execute it.
if constexpr (details::HasBasicFinalizer<T>::value) {
#ifndef NODE_API_EXPERIMENTAL_HAS_POST_FINALIZER
HandleScope scope(env);
#endif

instance->Finalize(Napi::BasicEnv(env));
}

// If class overrides the asynchronous finalizer, either schedule it or
// If class overrides the (extended) finalizer, either schedule it or
// execute it immediately (depending on experimental features enabled).
if constexpr (details::HasAsyncFinalizer<T>::value) {
if constexpr (details::HasExtendedFinalizer<T>::value) {
#ifdef NODE_API_EXPERIMENTAL_HAS_POST_FINALIZER
// In experimental, attach via node_api_post_finalizer.
// `PostFinalizeCallback` is responsible for deleting the `T* instance`,
Expand All @@ -5040,16 +5038,16 @@ inline void ObjectWrap<T>::FinalizeCallback(node_api_nogc_env env,
"ObjectWrap<T>::FinalizeCallback",
"node_api_post_finalizer failed");
#else
// In non-experimental, this `FinalizeCallback` already executes from
// outside the garbage collector. Execute the override directly.
// In non-experimental, this `FinalizeCallback` already executes from a
// non-basic environment. Execute the override directly.
// `PostFinalizeCallback` is responsible for deleting the `T* instance`,
// after calling the user-provided finalizer.
HandleScope scope(env);
PostFinalizeCallback(env, data, static_cast<void*>(nullptr));
#endif
}
// If the instance does _not_ have an asynchronous finalizer, delete the `T*
// instance` immediately.
// If the instance does _not_ override the (extended) finalizer, delete the
// `T* instance` immediately.
else {
delete instance;
}
Expand Down
15 changes: 7 additions & 8 deletions napi.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -318,9 +318,9 @@ class BasicEnv {
node_api_nogc_env _env;
#if NAPI_VERSION > 5
template <typename T>
static void DefaultAsyncFini(Env, T* data);
static void DefaultFini(Env, T* data);
template <typename DataType, typename HintType>
static void DefaultAsyncFiniWithHint(Env, DataType* data, HintType* hint);
static void DefaultFiniWithHint(Env, DataType* data, HintType* hint);
#endif // NAPI_VERSION > 5
public:
BasicEnv(node_api_nogc_env env);
Expand Down Expand Up @@ -355,17 +355,16 @@ class BasicEnv {
T* GetInstanceData() const;

template <typename T>
using AsyncFinalizer = void (*)(Env, T*);
template <typename T,
AsyncFinalizer<T> async_fini = BasicEnv::DefaultAsyncFini<T>>
using Finalizer = void (*)(Env, T*);
template <typename T, Finalizer<T> fini = BasicEnv::DefaultFini<T>>
void SetInstanceData(T* data) const;

template <typename DataType, typename HintType>
using AsyncFinalizerWithHint = void (*)(Env, DataType*, HintType*);
using FinalizerWithHint = void (*)(Env, DataType*, HintType*);
template <typename DataType,
typename HintType,
AsyncFinalizerWithHint<DataType, HintType> fini =
BasicEnv::DefaultAsyncFiniWithHint<DataType, HintType>>
FinalizerWithHint<DataType, HintType> fini =
BasicEnv::DefaultFiniWithHint<DataType, HintType>>
void SetInstanceData(DataType* data, HintType* hint) const;
#endif // NAPI_VERSION > 5

Expand Down
72 changes: 36 additions & 36 deletions test/finalizer_order.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ namespace {
class Test : public Napi::ObjectWrap<Test> {
public:
Test(const Napi::CallbackInfo& info) : Napi::ObjectWrap<Test>(info) {
syncFinalizerCalled = false;
asyncFinalizerCalled = false;
basicFinalizerCalled = false;
finalizerCalled = false;

if (info.Length() > 0) {
finalizeCb_ = Napi::Persistent(info[0].As<Napi::Function>());
Expand All @@ -17,71 +17,71 @@ class Test : public Napi::ObjectWrap<Test> {
DefineClass(env,
"Test",
{
StaticAccessor("isSyncFinalizerCalled",
&IsSyncFinalizerCalled,
StaticAccessor("isBasicFinalizerCalled",
&IsBasicFinalizerCalled,
nullptr,
napi_default),
StaticAccessor("isAsyncFinalizerCalled",
&IsAsyncFinalizerCalled,
StaticAccessor("isFinalizerCalled",
&IsFinalizerCalled,
nullptr,
napi_default),
}));
}

void Finalize(Napi::BasicEnv /*env*/) { syncFinalizerCalled = true; }
void Finalize(Napi::BasicEnv /*env*/) { basicFinalizerCalled = true; }

void Finalize(Napi::Env /*env*/) {
asyncFinalizerCalled = true;
finalizerCalled = true;
if (!finalizeCb_.IsEmpty()) {
finalizeCb_.Call({});
}
}

static Napi::Value IsSyncFinalizerCalled(const Napi::CallbackInfo& info) {
return Napi::Boolean::New(info.Env(), syncFinalizerCalled);
static Napi::Value IsBasicFinalizerCalled(const Napi::CallbackInfo& info) {
return Napi::Boolean::New(info.Env(), basicFinalizerCalled);
}

static Napi::Value IsAsyncFinalizerCalled(const Napi::CallbackInfo& info) {
return Napi::Boolean::New(info.Env(), asyncFinalizerCalled);
static Napi::Value IsFinalizerCalled(const Napi::CallbackInfo& info) {
return Napi::Boolean::New(info.Env(), finalizerCalled);
}

private:
Napi::FunctionReference finalizeCb_;

static bool syncFinalizerCalled;
static bool asyncFinalizerCalled;
static bool basicFinalizerCalled;
static bool finalizerCalled;
};

bool Test::syncFinalizerCalled = false;
bool Test::asyncFinalizerCalled = false;
bool Test::basicFinalizerCalled = false;
bool Test::finalizerCalled = false;

bool externalSyncFinalizerCalled = false;
bool externalAsyncFinalizerCalled = false;
bool externalBasicFinalizerCalled = false;
bool externalFinalizerCalled = false;

Napi::Value CreateExternalSyncFinalizer(const Napi::CallbackInfo& info) {
externalSyncFinalizerCalled = false;
Napi::Value CreateExternalBasicFinalizer(const Napi::CallbackInfo& info) {
externalBasicFinalizerCalled = false;
return Napi::External<int>::New(
info.Env(), new int(1), [](Napi::BasicEnv /*env*/, int* data) {
externalSyncFinalizerCalled = true;
externalBasicFinalizerCalled = true;
delete data;
});
}

Napi::Value CreateExternalAsyncFinalizer(const Napi::CallbackInfo& info) {
externalAsyncFinalizerCalled = false;
Napi::Value CreateExternalFinalizer(const Napi::CallbackInfo& info) {
externalFinalizerCalled = false;
return Napi::External<int>::New(
info.Env(), new int(1), [](Napi::Env /*env*/, int* data) {
externalAsyncFinalizerCalled = true;
externalFinalizerCalled = true;
delete data;
});
}

Napi::Value IsExternalSyncFinalizerCalled(const Napi::CallbackInfo& info) {
return Napi::Boolean::New(info.Env(), externalSyncFinalizerCalled);
Napi::Value isExternalBasicFinalizerCalled(const Napi::CallbackInfo& info) {
return Napi::Boolean::New(info.Env(), externalBasicFinalizerCalled);
}

Napi::Value IsExternalAsyncFinalizerCalled(const Napi::CallbackInfo& info) {
return Napi::Boolean::New(info.Env(), externalAsyncFinalizerCalled);
Napi::Value IsExternalFinalizerCalled(const Napi::CallbackInfo& info) {
return Napi::Boolean::New(info.Env(), externalFinalizerCalled);
}

#ifdef NODE_API_EXPERIMENTAL_HAS_POST_FINALIZER
Expand Down Expand Up @@ -132,14 +132,14 @@ Napi::Value PostFinalizerWithDataAndHint(const Napi::CallbackInfo& info) {
Napi::Object InitFinalizerOrder(Napi::Env env) {
Napi::Object exports = Napi::Object::New(env);
Test::Initialize(env, exports);
exports["createExternalSyncFinalizer"] =
Napi::Function::New(env, CreateExternalSyncFinalizer);
exports["createExternalAsyncFinalizer"] =
Napi::Function::New(env, CreateExternalAsyncFinalizer);
exports["isExternalSyncFinalizerCalled"] =
Napi::Function::New(env, IsExternalSyncFinalizerCalled);
exports["isExternalAsyncFinalizerCalled"] =
Napi::Function::New(env, IsExternalAsyncFinalizerCalled);
exports["createExternalBasicFinalizer"] =
Napi::Function::New(env, CreateExternalBasicFinalizer);
exports["createExternalFinalizer"] =
Napi::Function::New(env, CreateExternalFinalizer);
exports["isExternalBasicFinalizerCalled"] =
Napi::Function::New(env, isExternalBasicFinalizerCalled);
exports["isExternalFinalizerCalled"] =
Napi::Function::New(env, IsExternalFinalizerCalled);

#ifdef NODE_API_EXPERIMENTAL_HAS_POST_FINALIZER
exports["PostFinalizer"] = Napi::Function::New(env, PostFinalizer);
Expand Down
31 changes: 15 additions & 16 deletions test/finalizer_order.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,47 +22,46 @@ function test (binding) {
global.gc();

if (isExperimental) {
assert.strictEqual(binding.finalizer_order.Test.isSyncFinalizerCalled, true, 'Expected sync finalizer to be called [before ticking]');
assert.strictEqual(binding.finalizer_order.Test.isAsyncFinalizerCalled, false, 'Expected async finalizer to not be called [before ticking]');
assert.strictEqual(binding.finalizer_order.Test.isBasicFinalizerCalled, true, 'Expected basic finalizer to be called [before ticking]');
assert.strictEqual(binding.finalizer_order.Test.isFinalizerCalled, false, 'Expected (extended) finalizer to not be called [before ticking]');
assert.strictEqual(isCallbackCalled, false, 'Expected callback to not be called [before ticking]');
} else {
assert.strictEqual(binding.finalizer_order.Test.isSyncFinalizerCalled, false, 'Expected sync finalizer to not be called [before ticking]');
assert.strictEqual(binding.finalizer_order.Test.isAsyncFinalizerCalled, false, 'Expected async finalizer to not be called [before ticking]');
assert.strictEqual(binding.finalizer_order.Test.isBasicFinalizerCalled, false, 'Expected basic finalizer to not be called [before ticking]');
assert.strictEqual(binding.finalizer_order.Test.isFinalizerCalled, false, 'Expected (extended) finalizer to not be called [before ticking]');
assert.strictEqual(isCallbackCalled, false, 'Expected callback to not be called [before ticking]');
}
},
() => {
assert.strictEqual(binding.finalizer_order.Test.isSyncFinalizerCalled, true, 'Expected sync finalizer to be called [after ticking]');
assert.strictEqual(binding.finalizer_order.Test.isAsyncFinalizerCalled, true, 'Expected async finalizer to be called [after ticking]');
assert.strictEqual(binding.finalizer_order.Test.isBasicFinalizerCalled, true, 'Expected basic finalizer to be called [after ticking]');
assert.strictEqual(binding.finalizer_order.Test.isFinalizerCalled, true, 'Expected (extended) finalizer to be called [after ticking]');
assert.strictEqual(isCallbackCalled, true, 'Expected callback to be called [after ticking]');
},

'Finalizer Order - External with Sync Finalizer',
'Finalizer Order - External with Basic Finalizer',
() => {
console.log(binding.finalizer_order);
let ext = binding.finalizer_order.createExternalSyncFinalizer();
let ext = binding.finalizer_order.createExternalBasicFinalizer();
ext = null;
global.gc();

if (isExperimental) {
assert.strictEqual(binding.finalizer_order.isExternalSyncFinalizerCalled(), true, 'Expected External sync finalizer to be called [before ticking]');
assert.strictEqual(binding.finalizer_order.isExternalBasicFinalizerCalled(), true, 'Expected External basic finalizer to be called [before ticking]');
} else {
assert.strictEqual(binding.finalizer_order.isExternalSyncFinalizerCalled(), false, 'Expected External sync finalizer to not be called [before ticking]');
assert.strictEqual(binding.finalizer_order.isExternalBasicFinalizerCalled(), false, 'Expected External basic finalizer to not be called [before ticking]');
}
},
() => {
assert.strictEqual(binding.finalizer_order.isExternalSyncFinalizerCalled(), true, 'Expected External sync finalizer to be called [after ticking]');
assert.strictEqual(binding.finalizer_order.isExternalBasicFinalizerCalled(), true, 'Expected External basic finalizer to be called [after ticking]');
},

'Finalizer Order - External with Async Finalizer',
'Finalizer Order - External with Finalizer',
() => {
let ext = binding.finalizer_order.createExternalAsyncFinalizer();
let ext = binding.finalizer_order.createExternalFinalizer();
ext = null;
global.gc();
assert.strictEqual(binding.finalizer_order.isExternalAsyncFinalizerCalled(), false, 'Expected External async finalizer to not be called [before ticking]');
assert.strictEqual(binding.finalizer_order.isExternalFinalizerCalled(), false, 'Expected External extended finalizer to not be called [before ticking]');
},
() => {
assert.strictEqual(binding.finalizer_order.isExternalAsyncFinalizerCalled(), true, 'Expected External async finalizer to be called [after ticking]');
assert.strictEqual(binding.finalizer_order.isExternalFinalizerCalled(), true, 'Expected External extended finalizer to be called [after ticking]');
}
];

Expand Down

0 comments on commit 5ca9633

Please sign in to comment.