Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Segfault on Electron Exit #501

Merged
merged 35 commits into from
Nov 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
0b95abf
initial test
xiazhvera Jun 19, 2023
150996b
fix buffer reference
xiazhvera Jun 19, 2023
f363d94
fix release allocator error
xiazhvera Jun 19, 2023
00a5585
fix bug
xiazhvera Jun 19, 2023
6c65fe4
remove callbacks on close
xiazhvera Jul 5, 2023
2dcd890
Merge branch 'main' into no_external_buffers
xiazhvera Jul 5, 2023
6299d73
remove pre-compile flag
xiazhvera Jul 5, 2023
d30c3e4
Merge branch 'no_external_buffers' of https://github.com/awslabs/aws-…
xiazhvera Jul 5, 2023
2006ec7
fix finalize_cb
xiazhvera Jul 5, 2023
98e0b12
fix create external buffer
xiazhvera Jul 5, 2023
6722468
avoid use node14 enum
xiazhvera Jul 6, 2023
a567b5c
fix finalize callback
xiazhvera Jul 6, 2023
796aff7
remove error log for external arraybuffer failure
xiazhvera Jul 6, 2023
5e3aca1
fix local var name
xiazhvera Jul 6, 2023
8d32859
clean up codes..
xiazhvera Jul 6, 2023
6997ac7
update cr
xiazhvera Jul 12, 2023
d801db0
fix AWS_NAPI_ENSURE compile error
xiazhvera Jul 12, 2023
1446c09
clang-format
xiazhvera Jul 12, 2023
b6c7f7f
Merge branch 'main' of https://github.com/awslabs/aws-crt-nodejs into…
xiazhvera Oct 24, 2023
32e6067
Merge branch 'main' of https://github.com/awslabs/aws-crt-nodejs into…
xiazhvera Nov 2, 2023
85ab5c3
update cr
xiazhvera Nov 2, 2023
309a252
update log
xiazhvera Nov 2, 2023
0b6da1c
Merge branch 'main' into no_external_buffers
xiazhvera Nov 6, 2023
c391a60
add aws_threadsafe_function
xiazhvera Nov 6, 2023
c6abeb8
test on exit..
xiazhvera Nov 7, 2023
b9aac1a
use rw lock
xiazhvera Nov 7, 2023
44e10ad
clang-format
xiazhvera Nov 7, 2023
f9e9cd9
disalbe only in electron
xiazhvera Nov 7, 2023
1b347a2
update napi function export
xiazhvera Nov 7, 2023
0a4a4a4
remove unused file
xiazhvera Nov 7, 2023
c23c311
release the lock in a later time
xiazhvera Nov 7, 2023
edfeddf
update comments
xiazhvera Nov 7, 2023
452d6f5
release lock after all library released
xiazhvera Nov 7, 2023
7f4cab2
fix comments
xiazhvera Nov 9, 2023
33839ef
Merge branch 'main' of https://github.com/awslabs/aws-crt-nodejs into…
xiazhvera Nov 9, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions lib/native/binding.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ export function native_memory_dump(): void;
export function error_code_to_string(error_code: number): string;
/** @internal */
export function error_code_to_name(error_code: number): string;
/** @internal */
export function disable_threadsafe_function(): number;

/* IO */
/** @internal */
Expand Down
14 changes: 12 additions & 2 deletions lib/native/binding.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ const CRuntimeType = Object.freeze({
});

function getCRuntime() {
if(platform() !== "linux") {
if (platform() !== "linux") {
return CRuntimeType.NON_LINUX;
}

Expand Down Expand Up @@ -74,5 +74,15 @@ if (binding == undefined) {
throw new Error("AWS CRT binary not present in any of the following locations:\n\t" + search_paths.join('\n\t'));
}

import crt_native from "./binding"
/** Electron will shutdown the node process on exit, which causes the threadsafe function to segfault. To prevent
* the segfault we disable the threadsafe function on node process exit. */
if (process.versions.hasOwnProperty('electron')) {
process.on('exit', function () {
crt_native.disable_threadsafe_function();
});
}


export default binding;
export { CRuntimeType , cRuntime };
export { CRuntimeType, cRuntime };
109 changes: 84 additions & 25 deletions source/module.c
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include <aws/common/logging.h>
#include <aws/common/mutex.h>
#include <aws/common/ref_count.h>
#include <aws/common/rw_lock.h>
#include <aws/common/system_info.h>

#include <aws/event-stream/event_stream.h>
Expand All @@ -47,6 +48,15 @@ AWS_STATIC_ASSERT(NAPI_VERSION >= 4);

#define AWS_DEFINE_ERROR_INFO_CRT_NODEJS(CODE, STR) AWS_DEFINE_ERROR_INFO(CODE, STR, "aws-crt-nodejs")

/* TODO:
* Hardcoded enum value for `napi_no_external_buffers_allowed`.
* The enum `napi_no_external_buffers_allowed` is introduced in node14.
* Use it for external buffer related changes after bump to node 14 */
#define NAPI_NO_EXTERNAL_BUFFER_ENUM_VALUE 22

static bool s_tsfn_enabled = false;
static struct aws_rw_lock s_tsfn_lock;

/* clang-format off */
static struct aws_error_info s_errors[] = {
AWS_DEFINE_ERROR_INFO_CRT_NODEJS(
Expand Down Expand Up @@ -743,6 +753,28 @@ int aws_napi_get_property_array_size(
return AWS_OP_SUCCESS;
}

void s_aws_enable_threadsafe_function(void) {
aws_rw_lock_wlock(&s_tsfn_lock);
s_tsfn_enabled = true;
aws_rw_lock_wunlock(&s_tsfn_lock);
}

void s_aws_disable_threadsafe_function(void) {
aws_rw_lock_wlock(&s_tsfn_lock);
s_tsfn_enabled = false;
aws_rw_lock_wunlock(&s_tsfn_lock);
}

napi_value aws_napi_disable_threadsafe_function(napi_env env, napi_callback_info info) {
(void)info;
if (env == NULL) {
aws_raise_error(AWS_CRT_NODEJS_ERROR_THREADSAFE_FUNCTION_NULL_NAPI_ENV);
return NULL;
}
s_aws_disable_threadsafe_function();
return NULL;
}

void aws_napi_throw_last_error(napi_env env) {
const int error_code = aws_last_error();
napi_throw_error(env, aws_error_str(error_code), aws_error_debug_str(error_code));
Expand Down Expand Up @@ -975,19 +1007,25 @@ napi_status aws_napi_dispatch_threadsafe_function(
size_t argc,
napi_value *argv) {

napi_status call_status = napi_ok;
if (!this_ptr) {
AWS_NAPI_ENSURE(env, napi_get_undefined(env, &this_ptr));
aws_rw_lock_rlock(&s_tsfn_lock);
napi_status result = napi_ok;
if (s_tsfn_enabled) {
napi_status call_status = napi_ok;
if (!this_ptr) {
AWS_NAPI_ENSURE(env, napi_get_undefined(env, &this_ptr));
}
AWS_NAPI_CALL(env, napi_call_function(env, this_ptr, function, argc, argv, NULL), {
call_status = status;
s_handle_failed_callback(env, function, status);
});
/* main thread can exit now */
napi_unref_threadsafe_function(env, tsfn);
/* Must always decrement the ref count, or the function will be pinned */
napi_status release_status = napi_release_threadsafe_function(tsfn, napi_tsfn_release);
result = (call_status != napi_ok) ? call_status : release_status;
}
AWS_NAPI_CALL(env, napi_call_function(env, this_ptr, function, argc, argv, NULL), {
call_status = status;
s_handle_failed_callback(env, function, status);
});
/* main thread can exit now */
napi_unref_threadsafe_function(env, tsfn);
/* Must always decrement the ref count, or the function will be pinned */
napi_status release_status = napi_release_threadsafe_function(tsfn, napi_tsfn_release);
return (call_status != napi_ok) ? call_status : release_status;
aws_rw_lock_runlock(&s_tsfn_lock);
return result;
}

napi_status aws_napi_create_threadsafe_function(
Expand Down Expand Up @@ -1018,30 +1056,45 @@ napi_status aws_napi_create_threadsafe_function(
napi_status aws_napi_release_threadsafe_function(
napi_threadsafe_function function,
napi_threadsafe_function_release_mode mode) {
if (function) {
return napi_release_threadsafe_function(function, mode);
napi_status result = napi_ok;
aws_rw_lock_rlock(&s_tsfn_lock);
if (s_tsfn_enabled && function) {
result = napi_release_threadsafe_function(function, mode);
}
return napi_ok;
aws_rw_lock_runlock(&s_tsfn_lock);
return result;
}

napi_status aws_napi_acquire_threadsafe_function(napi_threadsafe_function function) {
if (function) {
return napi_acquire_threadsafe_function(function);
napi_status result = napi_ok;
aws_rw_lock_rlock(&s_tsfn_lock);
if (s_tsfn_enabled && function) {
result = napi_acquire_threadsafe_function(function);
}
return napi_ok;
aws_rw_lock_runlock(&s_tsfn_lock);
return result;
}

napi_status aws_napi_unref_threadsafe_function(napi_env env, napi_threadsafe_function function) {
if (function) {
return napi_unref_threadsafe_function(env, function);
napi_status result = napi_ok;
aws_rw_lock_rlock(&s_tsfn_lock);
if (s_tsfn_enabled && function) {
result = napi_unref_threadsafe_function(env, function);
}
return napi_ok;
aws_rw_lock_runlock(&s_tsfn_lock);
return result;
}

napi_status aws_napi_queue_threadsafe_function(napi_threadsafe_function function, void *user_data) {
/* increase the ref count, gets decreased when the call completes */
AWS_NAPI_ENSURE(NULL, napi_acquire_threadsafe_function(function));
return napi_call_threadsafe_function(function, user_data, napi_tsfn_nonblocking);
napi_status result = napi_ok;
aws_rw_lock_rlock(&s_tsfn_lock);
if (s_tsfn_enabled && function) {
/* increase the ref count, gets decreased when the call completes */
AWS_NAPI_ENSURE(NULL, napi_acquire_threadsafe_function(function));
result = napi_call_threadsafe_function(function, user_data, napi_tsfn_nonblocking);
}
aws_rw_lock_runlock(&s_tsfn_lock);
return result;
}

AWS_STATIC_STRING_FROM_LITERAL(s_mem_tracing_env_var, "AWS_CRT_MEMORY_TRACING");
Expand Down Expand Up @@ -1145,6 +1198,7 @@ static void s_napi_context_finalize(napi_env env, void *user_data, void *finaliz
--s_module_initialize_count;

if (s_module_initialize_count == 0) {

aws_client_bootstrap_release(s_default_client_bootstrap);
s_default_client_bootstrap = NULL;

Expand All @@ -1163,6 +1217,8 @@ static void s_napi_context_finalize(napi_env env, void *user_data, void *finaliz
aws_mqtt_library_clean_up();

s_uninstall_crash_handler();
// clean up threadsafe function lock
aws_rw_lock_clean_up(&s_tsfn_lock);
}

struct aws_napi_context *ctx = user_data;
Expand All @@ -1178,7 +1234,6 @@ static void s_napi_context_finalize(napi_env env, void *user_data, void *finaliz
aws_mem_tracer_destroy(ctx_allocator);
}
}

aws_mutex_unlock(&s_module_lock);
}

Expand Down Expand Up @@ -1230,6 +1285,9 @@ static bool s_create_and_register_function(
struct aws_allocator *allocator = aws_napi_get_allocator();

if (s_module_initialize_count == 0) {
aws_rw_lock_init(&s_tsfn_lock);
s_aws_enable_threadsafe_function();

s_install_crash_handler();

aws_mqtt_library_init(allocator);
Expand Down Expand Up @@ -1295,6 +1353,7 @@ static bool s_create_and_register_function(
CREATE_AND_REGISTER_FN(native_memory_dump)
CREATE_AND_REGISTER_FN(error_code_to_string)
CREATE_AND_REGISTER_FN(error_code_to_name)
CREATE_AND_REGISTER_FN(disable_threadsafe_function)

/* IO */
CREATE_AND_REGISTER_FN(io_logging_enable)
Expand Down
6 changes: 6 additions & 0 deletions source/module.h
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,12 @@ napi_status aws_napi_unref_threadsafe_function(napi_env env, napi_threadsafe_fun
*/
napi_status aws_napi_queue_threadsafe_function(napi_threadsafe_function function, void *user_data);

/**
* Disable the thread safe function operations. The function will prevent any access to threadsafe function
* including acquire, release, function call and so on.
*/
napi_value aws_napi_disable_threadsafe_function(napi_env env, napi_callback_info info);

/*
* One of these will be allocated each time the module init function is called
* Any global state that isn't thread safe or requires clean up should be stored
Expand Down