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 33 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
12 changes: 11 additions & 1 deletion 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"
/** As Electron would shutdown the node process on exit, and eventually causing "segfault" on threadsafe function operation.
xiazhvera marked this conversation as resolved.
Show resolved Hide resolved
* We disable the node threadsafe functions on node process exit to prevent the segfault */
if (process.versions.hasOwnProperty('electron')) {
process.on('exit', function () {
crt_native.disable_threadsafe_function();
});
}


export default binding;
export { CRuntimeType , cRuntime };
2 changes: 1 addition & 1 deletion source/http_stream.c
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ static void s_on_body_call(napi_env env, napi_value on_body, void *context, void

AWS_NAPI_ENSURE(
env,
napi_create_external_arraybuffer(
aws_napi_create_external_arraybuffer(
env, args->chunk.buffer, args->chunk.len, s_external_arraybuffer_finalizer, args, &params[0]));

AWS_NAPI_ENSURE(
Expand Down
151 changes: 123 additions & 28 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 @@ -268,9 +278,10 @@ int aws_napi_attach_object_property_binary_as_finalizable_external(
}

napi_value napi_binary = NULL;

AWS_NAPI_ENSURE(
env,
napi_create_external_arraybuffer(
aws_napi_create_external_arraybuffer(
env,
data_buffer->buffer,
data_buffer->len,
Expand Down Expand Up @@ -742,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 @@ -831,7 +864,6 @@ const char *aws_napi_status_to_str(napi_status status) {
case napi_callback_scope_mismatch:
reason = "napi_callback_scope_mismatch";
break;
#if NAPI_VERSION >= 3
case napi_queue_full:
reason = "napi_queue_full";
break;
Expand All @@ -841,7 +873,9 @@ const char *aws_napi_status_to_str(napi_status status) {
case napi_bigint_expected:
reason = "napi_bigint_expected";
break;
#endif
case NAPI_NO_EXTERNAL_BUFFER_ENUM_VALUE:
reason = "napi_no_external_buffers_allowed";
break;
}
return reason;
}
Expand Down Expand Up @@ -931,6 +965,40 @@ static void s_handle_failed_callback(napi_env env, napi_value function, napi_sta
}
}

napi_status aws_napi_create_external_arraybuffer(
napi_env env,
void *external_data,
size_t byte_length,
napi_finalize finalize_cb,
void *finalize_hint,
napi_value *result) {

napi_status external_buffer_status =
napi_create_external_arraybuffer(env, external_data, byte_length, finalize_cb, finalize_hint, result);

if (external_buffer_status == NAPI_NO_EXTERNAL_BUFFER_ENUM_VALUE) {

// The external buffer is disabled, manually copy the external_data into Node
void *napi_buf_data = NULL;
napi_status create_arraybuffer_status = napi_create_arraybuffer(env, byte_length, &napi_buf_data, result);

if (create_arraybuffer_status != napi_ok) {
AWS_NAPI_LOGF_ERROR(
"napi_create_arraybuffer (in aws_napi_create_external_arraybuffer) failed with : %s",
aws_napi_status_to_str(create_arraybuffer_status));
return create_arraybuffer_status;
}

memcpy(napi_buf_data, external_data, byte_length);

// As the data has been copied into the Node, invoke the finalize callback to make sure the
// data is released.
finalize_cb(env, finalize_hint, finalize_hint);
}

return napi_ok;
}

napi_status aws_napi_dispatch_threadsafe_function(
napi_env env,
napi_threadsafe_function tsfn,
Expand All @@ -939,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 @@ -982,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 @@ -1109,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 @@ -1127,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 @@ -1142,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 @@ -1194,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 @@ -1259,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
25 changes: 25 additions & 0 deletions source/module.h
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,25 @@ struct aws_client_bootstrap *aws_napi_get_default_client_bootstrap(void);

const char *aws_napi_status_to_str(napi_status status);

/*
* Wrapper around napi_create_external_arraybuffer,
* The function returns `napi_ok` if array buffer is created successfully in nodejs. Otherwise returns the error code.
* The user is responsible to release/proceed the `external_data` if the creation failed.
*
* `aws_napi_create_external_arraybuffer` handles the creation of the arraybuffer from the `external_data`. As
* some runtimes other than Node.js have dropped support for external buffers, the napi function call will fail in such
* case. If the call failed, the function will directly create an arraybuffer in Node and copy the data of external
* buffer into it. Once data copied, the `finalize_cb` will be immediately invoked to release the external data.
*
*/
napi_status aws_napi_create_external_arraybuffer(
napi_env env,
void *external_data,
size_t byte_length,
napi_finalize finalize_cb,
void *finalize_hint,
napi_value *result);

/**
* Gets the allocator used to allocate native resources in the node environment, should be used
* by all binding code in this extension
Expand Down Expand Up @@ -290,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
8 changes: 6 additions & 2 deletions source/mqtt_client_connection.c
Original file line number Diff line number Diff line change
Expand Up @@ -1424,15 +1424,17 @@ static void s_on_publish_call(napi_env env, napi_value on_publish, void *context

AWS_NAPI_ENSURE(
env, napi_create_string_utf8(env, (const char *)args->topic.buffer, args->topic.len, &params[0]));

AWS_NAPI_ENSURE(
env,
napi_create_external_arraybuffer(
aws_napi_create_external_arraybuffer(
env,
args->payload->buffer,
args->payload->len,
s_publish_external_arraybuffer_finalizer,
args->payload,
&params[1]));

AWS_NAPI_ENSURE(env, napi_get_boolean(env, args->dup, &params[2]));
AWS_NAPI_ENSURE(env, napi_create_int32(env, args->qos, &params[3]));
AWS_NAPI_ENSURE(env, napi_get_boolean(env, args->retain, &params[4]));
Expand Down Expand Up @@ -1672,15 +1674,17 @@ static void s_on_any_publish_call(napi_env env, napi_value on_publish, void *con

AWS_NAPI_ENSURE(
env, napi_create_string_utf8(env, aws_string_c_str(args->topic), args->topic->len, &params[0]));

AWS_NAPI_ENSURE(
env,
napi_create_external_arraybuffer(
aws_napi_create_external_arraybuffer(
env,
args->payload->buffer,
args->payload->len,
s_any_publish_external_arraybuffer_finalizer,
args->payload,
&params[1]));

AWS_NAPI_ENSURE(env, napi_get_boolean(env, args->dup, &params[2]));
AWS_NAPI_ENSURE(env, napi_create_int32(env, args->qos, &params[3]));
AWS_NAPI_ENSURE(env, napi_get_boolean(env, args->retain, &params[4]));
Expand Down