Skip to content

Commit

Permalink
node-api: verify cleanup hooks order
Browse files Browse the repository at this point in the history
Cleanup hooks are called before the environment shutdown finalizer
invocations.

PR-URL: #46692
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
  • Loading branch information
legendecas authored and danielleadams committed Jul 6, 2023
1 parent baada5d commit d8d2d33
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 2 deletions.
31 changes: 31 additions & 0 deletions test/node-api/test_async_cleanup_hook/binding.c
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include <stdlib.h>
#include "../../js-native-api/common.h"

static int cleanup_hook_count = 0;
static void MustNotCall(napi_async_cleanup_hook_handle hook, void* arg) {
assert(0);
}
Expand All @@ -21,17 +22,20 @@ static struct AsyncData* CreateAsyncData() {
}

static void AfterCleanupHookTwo(uv_handle_t* handle) {
cleanup_hook_count++;
struct AsyncData* data = (struct AsyncData*) handle->data;
napi_status status = napi_remove_async_cleanup_hook(data->handle);
assert(status == napi_ok);
free(data);
}

static void AfterCleanupHookOne(uv_async_t* async) {
cleanup_hook_count++;
uv_close((uv_handle_t*) async, AfterCleanupHookTwo);
}

static void AsyncCleanupHook(napi_async_cleanup_hook_handle handle, void* arg) {
cleanup_hook_count++;
struct AsyncData* data = (struct AsyncData*) arg;
uv_loop_t* loop;
napi_status status = napi_get_uv_event_loop(data->env, &loop);
Expand All @@ -44,7 +48,31 @@ static void AsyncCleanupHook(napi_async_cleanup_hook_handle handle, void* arg) {
uv_async_send(&data->async);
}

static void ObjectFinalizer(napi_env env, void* data, void* hint) {
// AsyncCleanupHook and its subsequent callbacks are called twice.
assert(cleanup_hook_count == 6);

napi_ref* ref = data;
NODE_API_CALL_RETURN_VOID(env, napi_delete_reference(env, *ref));
free(ref);
}

static void CreateObjectWrap(napi_env env) {
napi_value js_obj;
napi_ref* ref = malloc(sizeof(*ref));
NODE_API_CALL_RETURN_VOID(env, napi_create_object(env, &js_obj));
NODE_API_CALL_RETURN_VOID(
env, napi_wrap(env, js_obj, ref, ObjectFinalizer, NULL, ref));
// create a strong reference so that the finalizer is called at shutdown.
NODE_API_CALL_RETURN_VOID(env, napi_reference_ref(env, *ref, NULL));
}

static napi_value Init(napi_env env, napi_value exports) {
// Reinitialize the static variable to be compatible with musl libc.
cleanup_hook_count = 0;
// Create object wrap before cleanup hooks.
CreateObjectWrap(env);

{
struct AsyncData* data = CreateAsyncData();
data->env = env;
Expand All @@ -64,6 +92,9 @@ static napi_value Init(napi_env env, napi_value exports) {
napi_remove_async_cleanup_hook(must_not_call_handle);
}

// Create object wrap after cleanup hooks.
CreateObjectWrap(env);

return NULL;
}

Expand Down
31 changes: 30 additions & 1 deletion test/node-api/test_cleanup_hook/binding.c
Original file line number Diff line number Diff line change
@@ -1,19 +1,48 @@
#include <assert.h>
#include <stdlib.h>
#include "../../js-native-api/common.h"
#include "node_api.h"
#include "uv.h"
#include "../../js-native-api/common.h"

static int cleanup_hook_count = 0;
static void cleanup(void* arg) {
cleanup_hook_count++;
printf("cleanup(%d)\n", *(int*)(arg));
}

static int secret = 42;
static int wrong_secret = 17;

static void ObjectFinalizer(napi_env env, void* data, void* hint) {
// cleanup is called once.
assert(cleanup_hook_count == 1);

napi_ref* ref = data;
NODE_API_CALL_RETURN_VOID(env, napi_delete_reference(env, *ref));
free(ref);
}

static void CreateObjectWrap(napi_env env) {
napi_value js_obj;
napi_ref* ref = malloc(sizeof(*ref));
NODE_API_CALL_RETURN_VOID(env, napi_create_object(env, &js_obj));
NODE_API_CALL_RETURN_VOID(
env, napi_wrap(env, js_obj, ref, ObjectFinalizer, NULL, ref));
// create a strong reference so that the finalizer is called at shutdown.
NODE_API_CALL_RETURN_VOID(env, napi_reference_ref(env, *ref, NULL));
}

static napi_value Init(napi_env env, napi_value exports) {
// Create object wrap before cleanup hooks.
CreateObjectWrap(env);

napi_add_env_cleanup_hook(env, cleanup, &wrong_secret);
napi_add_env_cleanup_hook(env, cleanup, &secret);
napi_remove_env_cleanup_hook(env, cleanup, &wrong_secret);

// Create object wrap after cleanup hooks.
CreateObjectWrap(env);

return NULL;
}

Expand Down
3 changes: 2 additions & 1 deletion test/node-api/test_cleanup_hook/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ const child_process = require('child_process');
if (process.argv[2] === 'child') {
require(`./build/${common.buildType}/binding`);
} else {
const { stdout } =
const { stdout, status, signal } =
child_process.spawnSync(process.execPath, [__filename, 'child']);
assert.strictEqual(status, 0, `process exited with status(${status}) and signal(${signal})`);
assert.strictEqual(stdout.toString().trim(), 'cleanup(42)');
}

0 comments on commit d8d2d33

Please sign in to comment.