Skip to content

Commit aafdfee

Browse files
authored
Let's try ref counting (#451)
Revert the changes that prevented multi-init of the NAPI module and add ref-counting support and careful, matching shutdown when the last ref goes away.
1 parent a33df90 commit aafdfee

File tree

1 file changed

+92
-73
lines changed

1 file changed

+92
-73
lines changed

source/module.c

Lines changed: 92 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -1074,31 +1074,63 @@ static void s_install_crash_handler(void) {
10741074
#endif
10751075
}
10761076

1077+
static void s_uninstall_crash_handler(void) {
1078+
#if defined(_WIN32)
1079+
SetUnhandledExceptionFilter(NULL);
1080+
#elif defined(AWS_HAVE_EXECINFO)
1081+
sigaction(SIGSEGV, NULL, NULL);
1082+
sigaction(SIGABRT, NULL, NULL);
1083+
sigaction(SIGILL, NULL, NULL);
1084+
sigaction(SIGBUS, NULL, NULL);
1085+
#endif
1086+
}
1087+
1088+
static struct aws_mutex s_module_lock = AWS_MUTEX_INIT;
1089+
static uint32_t s_module_initialize_count = 0;
1090+
10771091
static void s_napi_context_finalize(napi_env env, void *user_data, void *finalize_hint) {
10781092
(void)env;
10791093
(void)finalize_hint;
1080-
aws_client_bootstrap_release(s_default_client_bootstrap);
1081-
aws_host_resolver_release(s_default_host_resolver);
1082-
aws_event_loop_group_release(s_node_uv_elg);
10831094

1084-
aws_thread_join_all_managed();
1095+
aws_mutex_lock(&s_module_lock);
1096+
AWS_FATAL_ASSERT(s_module_initialize_count > 0);
1097+
--s_module_initialize_count;
1098+
1099+
if (s_module_initialize_count == 0) {
1100+
aws_client_bootstrap_release(s_default_client_bootstrap);
1101+
s_default_client_bootstrap = NULL;
1102+
1103+
aws_host_resolver_release(s_default_host_resolver);
1104+
s_default_host_resolver = NULL;
1105+
1106+
aws_event_loop_group_release(s_node_uv_elg);
1107+
s_node_uv_elg = NULL;
10851108

1086-
aws_unregister_log_subject_info_list(&s_log_subject_list);
1087-
aws_unregister_error_info(&s_error_list);
1088-
aws_auth_library_clean_up();
1089-
aws_mqtt_library_clean_up();
1109+
aws_thread_join_all_managed();
1110+
1111+
aws_unregister_log_subject_info_list(&s_log_subject_list);
1112+
aws_unregister_error_info(&s_error_list);
1113+
aws_auth_library_clean_up();
1114+
aws_mqtt_library_clean_up();
1115+
1116+
s_uninstall_crash_handler();
1117+
}
10901118

10911119
struct aws_napi_context *ctx = user_data;
10921120
aws_napi_logger_destroy(ctx->logger);
10931121
struct aws_allocator *ctx_allocator = ctx->allocator;
10941122
aws_mem_release(ctx->allocator, ctx);
10951123

1096-
if (ctx_allocator != aws_default_allocator()) {
1097-
if (s_allocator == ctx_allocator) {
1098-
s_allocator = NULL;
1124+
if (s_module_initialize_count == 0) {
1125+
if (ctx_allocator != aws_default_allocator()) {
1126+
if (s_allocator == ctx_allocator) {
1127+
s_allocator = NULL;
1128+
}
1129+
aws_mem_tracer_destroy(ctx_allocator);
10991130
}
1100-
aws_mem_tracer_destroy(ctx_allocator);
11011131
}
1132+
1133+
aws_mutex_unlock(&s_module_lock);
11021134
}
11031135

11041136
static struct aws_napi_context *s_napi_context_new(struct aws_allocator *allocator, napi_env env, napi_value exports) {
@@ -1136,76 +1168,63 @@ static bool s_create_and_register_function(
11361168
return true;
11371169
}
11381170

1139-
/*
1140-
* Temporary hack to detect multi-init so we can throw an exception because we haven't figured out the right way
1141-
* to support it yet. Better than a hard crash in native code.
1142-
*/
1143-
static struct aws_mutex s_module_lock = AWS_MUTEX_INIT;
1144-
static bool s_module_initialized = false;
1145-
11461171
/* napi_value */ NAPI_MODULE_INIT() /* (napi_env env, napi_value exports) */ {
11471172

1148-
bool already_initialized = false;
11491173
aws_mutex_lock(&s_module_lock);
1150-
if (s_module_initialized) {
1151-
already_initialized = true;
1152-
}
1153-
s_module_initialized = true;
1154-
aws_mutex_unlock(&s_module_lock);
1155-
1156-
if (already_initialized) {
1157-
napi_throw_error(env, NULL, "Aws-crt-nodejs does not yet support multi-initialization.");
1158-
return NULL;
1159-
}
1160-
1161-
s_install_crash_handler();
11621174

11631175
struct aws_allocator *allocator = aws_napi_get_allocator();
1164-
/* context is bound to exports, will be cleaned up by finalizer */
1165-
s_napi_context_new(allocator, env, exports);
1166-
1167-
aws_cal_library_init(allocator);
1168-
aws_http_library_init(allocator);
1169-
aws_mqtt_library_init(allocator);
1170-
aws_auth_library_init(allocator);
1171-
aws_register_error_info(&s_error_list);
1172-
aws_register_log_subject_info_list(&s_log_subject_list);
1173-
1174-
/* Initialize the event loop group */
1175-
/*
1176-
* We don't currently support multi-init of the module, but we should.
1177-
* Things that would need to be solved:
1178-
* (1) global objects (event loop group, logger, allocator, more)
1179-
* (2) multi-init/multi-cleanup of aws-c-*
1180-
* (3) allocator cross-talk/lifetimes
1181-
*/
1182-
AWS_FATAL_ASSERT(s_node_uv_elg == NULL);
1183-
s_node_uv_elg = aws_event_loop_group_new_default(allocator, 1, NULL);
1184-
AWS_FATAL_ASSERT(s_node_uv_elg != NULL);
1185-
1186-
/*
1187-
* Default host resolver and client bootstrap to use if none specific at the javascript level. In most
1188-
* cases the user doesn't even need to know about these, so let's let them leave it out completely.
1189-
*/
1190-
AWS_FATAL_ASSERT(s_default_host_resolver == NULL);
11911176

1192-
struct aws_host_resolver_default_options resolver_options = {
1193-
.max_entries = 64,
1194-
.el_group = s_node_uv_elg,
1195-
};
1196-
s_default_host_resolver = aws_host_resolver_new_default(allocator, &resolver_options);
1197-
AWS_FATAL_ASSERT(s_default_host_resolver != NULL);
1198-
1199-
AWS_FATAL_ASSERT(s_default_client_bootstrap == NULL);
1177+
if (s_module_initialize_count == 0) {
1178+
s_install_crash_handler();
1179+
1180+
aws_mqtt_library_init(allocator);
1181+
aws_auth_library_init(allocator);
1182+
aws_register_error_info(&s_error_list);
1183+
aws_register_log_subject_info_list(&s_log_subject_list);
1184+
1185+
/* Initialize the event loop group */
1186+
/*
1187+
* We don't currently support multi-init of the module, but we should.
1188+
* Things that would need to be solved:
1189+
* (1) global objects (event loop group, logger, allocator, more)
1190+
* (2) multi-init/multi-cleanup of aws-c-*
1191+
* (3) allocator cross-talk/lifetimes
1192+
*/
1193+
AWS_FATAL_ASSERT(s_node_uv_elg == NULL);
1194+
s_node_uv_elg = aws_event_loop_group_new_default(allocator, 1, NULL);
1195+
AWS_FATAL_ASSERT(s_node_uv_elg != NULL);
1196+
1197+
/*
1198+
* Default host resolver and client bootstrap to use if none specific at the javascript level. In most
1199+
* cases the user doesn't even need to know about these, so let's let them leave it out completely.
1200+
*/
1201+
AWS_FATAL_ASSERT(s_default_host_resolver == NULL);
1202+
1203+
struct aws_host_resolver_default_options resolver_options = {
1204+
.max_entries = 64,
1205+
.el_group = s_node_uv_elg,
1206+
};
1207+
s_default_host_resolver = aws_host_resolver_new_default(allocator, &resolver_options);
1208+
AWS_FATAL_ASSERT(s_default_host_resolver != NULL);
1209+
1210+
AWS_FATAL_ASSERT(s_default_client_bootstrap == NULL);
1211+
1212+
struct aws_client_bootstrap_options bootstrap_options = {
1213+
.event_loop_group = s_node_uv_elg,
1214+
.host_resolver = s_default_host_resolver,
1215+
};
1216+
1217+
s_default_client_bootstrap = aws_client_bootstrap_new(allocator, &bootstrap_options);
1218+
1219+
AWS_FATAL_ASSERT(s_default_client_bootstrap != NULL);
1220+
}
12001221

1201-
struct aws_client_bootstrap_options bootstrap_options = {
1202-
.event_loop_group = s_node_uv_elg,
1203-
.host_resolver = s_default_host_resolver,
1204-
};
1222+
++s_module_initialize_count;
12051223

1206-
s_default_client_bootstrap = aws_client_bootstrap_new(allocator, &bootstrap_options);
1224+
aws_mutex_unlock(&s_module_lock);
12071225

1208-
AWS_FATAL_ASSERT(s_default_client_bootstrap != NULL);
1226+
/* context is bound to exports, will be cleaned up by finalizer */
1227+
s_napi_context_new(allocator, env, exports);
12091228

12101229
napi_value null;
12111230
napi_get_null(env, &null);

0 commit comments

Comments
 (0)