Skip to content

Commit b3fe2ba

Browse files
legendecastargos
authored andcommitted
node-api: verify cleanup hooks order
Cleanup hooks are called before the environment shutdown finalizer invocations. PR-URL: #46692 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michael Dawson <midawson@redhat.com>
1 parent b0aad34 commit b3fe2ba

File tree

3 files changed

+63
-2
lines changed

3 files changed

+63
-2
lines changed

test/node-api/test_async_cleanup_hook/binding.c

+31
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#include <stdlib.h>
55
#include "../../js-native-api/common.h"
66

7+
static int cleanup_hook_count = 0;
78
static void MustNotCall(napi_async_cleanup_hook_handle hook, void* arg) {
89
assert(0);
910
}
@@ -21,17 +22,20 @@ static struct AsyncData* CreateAsyncData() {
2122
}
2223

2324
static void AfterCleanupHookTwo(uv_handle_t* handle) {
25+
cleanup_hook_count++;
2426
struct AsyncData* data = (struct AsyncData*) handle->data;
2527
napi_status status = napi_remove_async_cleanup_hook(data->handle);
2628
assert(status == napi_ok);
2729
free(data);
2830
}
2931

3032
static void AfterCleanupHookOne(uv_async_t* async) {
33+
cleanup_hook_count++;
3134
uv_close((uv_handle_t*) async, AfterCleanupHookTwo);
3235
}
3336

3437
static void AsyncCleanupHook(napi_async_cleanup_hook_handle handle, void* arg) {
38+
cleanup_hook_count++;
3539
struct AsyncData* data = (struct AsyncData*) arg;
3640
uv_loop_t* loop;
3741
napi_status status = napi_get_uv_event_loop(data->env, &loop);
@@ -44,7 +48,31 @@ static void AsyncCleanupHook(napi_async_cleanup_hook_handle handle, void* arg) {
4448
uv_async_send(&data->async);
4549
}
4650

51+
static void ObjectFinalizer(napi_env env, void* data, void* hint) {
52+
// AsyncCleanupHook and its subsequent callbacks are called twice.
53+
assert(cleanup_hook_count == 6);
54+
55+
napi_ref* ref = data;
56+
NODE_API_CALL_RETURN_VOID(env, napi_delete_reference(env, *ref));
57+
free(ref);
58+
}
59+
60+
static void CreateObjectWrap(napi_env env) {
61+
napi_value js_obj;
62+
napi_ref* ref = malloc(sizeof(*ref));
63+
NODE_API_CALL_RETURN_VOID(env, napi_create_object(env, &js_obj));
64+
NODE_API_CALL_RETURN_VOID(
65+
env, napi_wrap(env, js_obj, ref, ObjectFinalizer, NULL, ref));
66+
// create a strong reference so that the finalizer is called at shutdown.
67+
NODE_API_CALL_RETURN_VOID(env, napi_reference_ref(env, *ref, NULL));
68+
}
69+
4770
static napi_value Init(napi_env env, napi_value exports) {
71+
// Reinitialize the static variable to be compatible with musl libc.
72+
cleanup_hook_count = 0;
73+
// Create object wrap before cleanup hooks.
74+
CreateObjectWrap(env);
75+
4876
{
4977
struct AsyncData* data = CreateAsyncData();
5078
data->env = env;
@@ -64,6 +92,9 @@ static napi_value Init(napi_env env, napi_value exports) {
6492
napi_remove_async_cleanup_hook(must_not_call_handle);
6593
}
6694

95+
// Create object wrap after cleanup hooks.
96+
CreateObjectWrap(env);
97+
6798
return NULL;
6899
}
69100

test/node-api/test_cleanup_hook/binding.c

+30-1
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,48 @@
1+
#include <assert.h>
2+
#include <stdlib.h>
3+
#include "../../js-native-api/common.h"
14
#include "node_api.h"
25
#include "uv.h"
3-
#include "../../js-native-api/common.h"
46

7+
static int cleanup_hook_count = 0;
58
static void cleanup(void* arg) {
9+
cleanup_hook_count++;
610
printf("cleanup(%d)\n", *(int*)(arg));
711
}
812

913
static int secret = 42;
1014
static int wrong_secret = 17;
1115

16+
static void ObjectFinalizer(napi_env env, void* data, void* hint) {
17+
// cleanup is called once.
18+
assert(cleanup_hook_count == 1);
19+
20+
napi_ref* ref = data;
21+
NODE_API_CALL_RETURN_VOID(env, napi_delete_reference(env, *ref));
22+
free(ref);
23+
}
24+
25+
static void CreateObjectWrap(napi_env env) {
26+
napi_value js_obj;
27+
napi_ref* ref = malloc(sizeof(*ref));
28+
NODE_API_CALL_RETURN_VOID(env, napi_create_object(env, &js_obj));
29+
NODE_API_CALL_RETURN_VOID(
30+
env, napi_wrap(env, js_obj, ref, ObjectFinalizer, NULL, ref));
31+
// create a strong reference so that the finalizer is called at shutdown.
32+
NODE_API_CALL_RETURN_VOID(env, napi_reference_ref(env, *ref, NULL));
33+
}
34+
1235
static napi_value Init(napi_env env, napi_value exports) {
36+
// Create object wrap before cleanup hooks.
37+
CreateObjectWrap(env);
38+
1339
napi_add_env_cleanup_hook(env, cleanup, &wrong_secret);
1440
napi_add_env_cleanup_hook(env, cleanup, &secret);
1541
napi_remove_env_cleanup_hook(env, cleanup, &wrong_secret);
1642

43+
// Create object wrap after cleanup hooks.
44+
CreateObjectWrap(env);
45+
1746
return NULL;
1847
}
1948

test/node-api/test_cleanup_hook/test.js

+2-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,8 @@ const child_process = require('child_process');
66
if (process.argv[2] === 'child') {
77
require(`./build/${common.buildType}/binding`);
88
} else {
9-
const { stdout } =
9+
const { stdout, status, signal } =
1010
child_process.spawnSync(process.execPath, [__filename, 'child']);
11+
assert.strictEqual(status, 0, `process exited with status(${status}) and signal(${signal})`);
1112
assert.strictEqual(stdout.toString().trim(), 'cleanup(42)');
1213
}

0 commit comments

Comments
 (0)