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.

Sign up for GitHub

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

Main thread spread exception when thread-mgr enabled #1889

Merged
merged 20 commits into from
Jan 20, 2023
Merged

Main thread spread exception when thread-mgr enabled #1889

Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
3d416ab
main thread spread exception
xujuntwt95329 Jan 15, 2023
9067254
terminate threads when spread exception
xujuntwt95329 Jan 16, 2023
c72dfc9
add file header
xujuntwt95329 Jan 16, 2023
4174215
auto format
xujuntwt95329 Jan 16, 2023
2d3da36
spread exception during throwing
xujuntwt95329 Jan 16, 2023
ff1bd10
restore unused modification
xujuntwt95329 Jan 16, 2023
a4a2bb6
minor fix
xujuntwt95329 Jan 16, 2023
0016228
refactor wasi proc exception processing
xujuntwt95329 Jan 16, 2023
2426fd8
minor fix
xujuntwt95329 Jan 16, 2023
fc5c945
fix build error
xujuntwt95329 Jan 16, 2023
c770575
fix typo
xujuntwt95329 Jan 16, 2023
12cf0af
address some PR comments
xujuntwt95329 Jan 16, 2023
8f0a046
add lock when traversing list in thread-mgr
xujuntwt95329 Jan 16, 2023
1ace00f
avoid creating new thread while spreading exception
xujuntwt95329 Jan 16, 2023
d0f5d94
fix dead lock issue
xujuntwt95329 Jan 16, 2023
d7d78c0
update comments
xujuntwt95329 Jan 16, 2023
f30f900
check return value
xujuntwt95329 Jan 16, 2023
1d6553e
fix spec test issue
xujuntwt95329 Jan 17, 2023
315985b
fix sgx spec test issue
xujuntwt95329 Jan 17, 2023
d939080
address PR comments
xujuntwt95329 Jan 19, 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
39 changes: 6 additions & 33 deletions core/iwasm/aot/aot_runtime.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -899,24 +899,6 @@ create_exports(AOTModuleInstance *module_inst, AOTModule *module,
return create_export_funcs(module_inst, module, error_buf, error_buf_size);
}

static bool
clear_wasi_proc_exit_exception(AOTModuleInstance *module_inst)
{
#if WASM_ENABLE_LIBC_WASI != 0
const char *exception = aot_get_exception(module_inst);
if (exception && !strcmp(exception, "Exception: wasi proc exit")) {
/* The "wasi proc exit" exception is thrown by native lib to
let wasm app exit, which is a normal behavior, we clear
the exception here. */
aot_set_exception(module_inst, NULL);
return true;
}
return false;
#else
return false;
#endif
}

static bool
execute_post_inst_function(AOTModuleInstance *module_inst)
{
Expand All @@ -931,6 +913,9 @@ execute_post_inst_function(AOTModuleInstance *module_inst)
NULL);
}

extern bool
clear_wasi_proc_exit_exception(WASMModuleInstanceCommon *module_inst_comm);

static bool
execute_start_function(AOTModuleInstance *module_inst)
{
Expand All @@ -956,7 +941,8 @@ execute_start_function(AOTModuleInstance *module_inst)
u.f(exec_env);

wasm_exec_env_destroy(exec_env);
(void)clear_wasi_proc_exit_exception(module_inst);
(void)clear_wasi_proc_exit_exception(
(WASMModuleInstanceCommon *)module_inst);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems no need to clear wasi proc exit exception here. WASI module doesn't set internal start func index，instead it exports a function named "_start". If here it really call wasi proc exit，we can also let instantiation process failed. @lum1n0us what is your opinion?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree with that, and updated

return !aot_get_exception(module_inst);
}

Expand Down Expand Up @@ -1407,13 +1393,6 @@ aot_call_function(WASMExecEnv *exec_env, AOTFunctionInstance *function,
ret = invoke_native_internal(exec_env, function->u.func.func_ptr,
func_type, NULL, NULL, argv1, argc, argv);

if (!ret || aot_get_exception(module_inst)) {
if (clear_wasi_proc_exit_exception(module_inst))
ret = true;
else
ret = false;
}

#if WASM_ENABLE_DUMP_CALL_STACK != 0
if (!ret) {
if (aot_create_call_stack(exec_env)) {
Expand Down Expand Up @@ -1473,9 +1452,6 @@ aot_call_function(WASMExecEnv *exec_env, AOTFunctionInstance *function,
ret = invoke_native_internal(exec_env, function->u.func.func_ptr,
func_type, NULL, NULL, argv, argc, argv);

if (clear_wasi_proc_exit_exception(module_inst))
ret = true;

#if WASM_ENABLE_DUMP_CALL_STACK != 0
if (aot_get_exception(module_inst)) {
if (aot_create_call_stack(exec_env)) {
Expand Down Expand Up @@ -1516,7 +1492,7 @@ aot_create_exec_env_and_call_function(AOTModuleInstance *module_inst,
}
}

ret = aot_call_function(exec_env, func, argc, argv);
ret = wasm_runtime_call_wasm(exec_env, func, argc, argv);

/* don't destroy the exec_env if it isn't created in this function */
if (!existing_exec_env)
Expand Down Expand Up @@ -2006,9 +1982,6 @@ aot_call_indirect(WASMExecEnv *exec_env, uint32 tbl_idx, uint32 table_elem_idx,
}

fail:
if (clear_wasi_proc_exit_exception(module_inst))
return true;

#ifdef OS_ENABLE_HW_BOUND_CHECK
wasm_runtime_access_exce_check_guard_page();
#endif
Expand Down
3 changes: 3 additions & 0 deletions core/iwasm/common/wasm_exec_env.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,9 @@ wasm_exec_env_destroy(WASMExecEnv *exec_env)
the stopped thread will be overrided by other threads */
wasm_cluster_thread_exited(exec_env);
#endif
/* We have terminated other threads, this is the only alive thread, so
* we don't acquire cluster->lock because the cluster will be destroyed
* inside this function */
wasm_cluster_del_exec_env(cluster, exec_env);
}
#endif /* end of WASM_ENABLE_THREAD_MGR */
Expand Down
66 changes: 58 additions & 8 deletions core/iwasm/common/wasm_runtime_common.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1743,6 +1743,30 @@ wasm_runtime_finalize_call_function(WASMExecEnv *exec_env,
}
#endif

bool
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can set to static if the above is ok

clear_wasi_proc_exit_exception(WASMModuleInstanceCommon *module_inst_comm)
{
#if WASM_ENABLE_LIBC_WASI != 0
const char *exception;
WASMModuleInstance *module_inst = (WASMModuleInstance *)module_inst_comm;

bh_assert(module_inst_comm->module_type == Wasm_Module_Bytecode
|| module_inst_comm->module_type == Wasm_Module_AoT);

exception = wasm_get_exception(module_inst);
if (exception && !strcmp(exception, "Exception: wasi proc exit")) {
/* The "wasi proc exit" exception is thrown by native lib to
let wasm app exit, which is a normal behavior, we clear
the exception here. */
wasm_set_exception(module_inst, NULL);
return true;
}
return false;
#else
return false;
#endif
}

bool
wasm_runtime_call_wasm(WASMExecEnv *exec_env,
WASMFunctionInstanceCommon *function, uint32 argc,
Expand Down Expand Up @@ -1783,10 +1807,15 @@ wasm_runtime_call_wasm(WASMExecEnv *exec_env,
param_argc, new_argv);
#endif
if (!ret) {
if (new_argv != argv) {
wasm_runtime_free(new_argv);
if (clear_wasi_proc_exit_exception(exec_env->module_inst)) {
ret = true;
}
else {
if (new_argv != argv) {
wasm_runtime_free(new_argv);
}
return false;
}
return false;
}

#if WASM_ENABLE_REF_TYPES != 0
Expand Down Expand Up @@ -2150,11 +2179,25 @@ wasm_runtime_get_exec_env_singleton(WASMModuleInstanceCommon *module_inst_comm)
void
wasm_set_exception(WASMModuleInstance *module_inst, const char *exception)
{
if (exception)
WASMExecEnv *exec_env = NULL;

if (exception) {
snprintf(module_inst->cur_exception, sizeof(module_inst->cur_exception),
"Exception: %s", exception);
else
}
else {
module_inst->cur_exception[0] = '\0';
}

#if WASM_ENABLE_THREAD_MGR != 0
exec_env =
wasm_clusters_search_exec_env((WASMModuleInstanceCommon *)module_inst);
if (exec_env) {
wasm_cluster_spread_exception(exec_env, exception ? false : true);
}
#else
(void)exec_env;
#endif
}

/* clang-format off */
Expand Down Expand Up @@ -4179,6 +4222,8 @@ bool
wasm_runtime_call_indirect(WASMExecEnv *exec_env, uint32 element_indices,
uint32 argc, uint32 argv[])
{
bool ret = false;

if (!wasm_runtime_exec_env_check(exec_env)) {
LOG_ERROR("Invalid exec env stack info.");
return false;
Expand All @@ -4190,13 +4235,18 @@ wasm_runtime_call_indirect(WASMExecEnv *exec_env, uint32 element_indices,

#if WASM_ENABLE_INTERP != 0
if (exec_env->module_inst->module_type == Wasm_Module_Bytecode)
return wasm_call_indirect(exec_env, 0, element_indices, argc, argv);
ret = wasm_call_indirect(exec_env, 0, element_indices, argc, argv);
#endif
#if WASM_ENABLE_AOT != 0
if (exec_env->module_inst->module_type == Wasm_Module_AoT)
return aot_call_indirect(exec_env, 0, element_indices, argc, argv);
ret = aot_call_indirect(exec_env, 0, element_indices, argc, argv);
#endif
return false;

if (!ret && clear_wasi_proc_exit_exception(exec_env->module_inst)) {
ret = true;
}

return ret;
}

static void
Expand Down
29 changes: 0 additions & 29 deletions core/iwasm/interpreter/wasm_interp_classic.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4018,24 +4018,6 @@ fast_jit_call_func_bytecode(WASMModuleInstance *module_inst,
#endif /* end of WASM_ENABLE_FAST_JIT != 0 */

#if WASM_ENABLE_JIT != 0
static bool
clear_wasi_proc_exit_exception(WASMModuleInstance *module_inst)
{
#if WASM_ENABLE_LIBC_WASI != 0
const char *exception = wasm_get_exception(module_inst);
if (exception && !strcmp(exception, "Exception: wasi proc exit")) {
/* The "wasi proc exit" exception is thrown by native lib to
let wasm app exit, which is a normal behavior, we clear
the exception here. */
wasm_set_exception(module_inst, NULL);
return true;
}
return false;
#else
return false;
#endif
}

static bool
llvm_jit_call_func_bytecode(WASMModuleInstance *module_inst,
WASMExecEnv *exec_env,
Expand Down Expand Up @@ -4095,14 +4077,6 @@ llvm_jit_call_func_bytecode(WASMModuleInstance *module_inst,
ret = wasm_runtime_invoke_native(
exec_env, module_inst->func_ptrs[func_idx], func_type, NULL, NULL,
argv1, argc, argv);

if (!ret || wasm_get_exception(module_inst)) {
if (clear_wasi_proc_exit_exception(module_inst))
ret = true;
else
ret = false;
}

if (!ret) {
if (argv1 != argv1_buf)
wasm_runtime_free(argv1);
Expand Down Expand Up @@ -4147,9 +4121,6 @@ llvm_jit_call_func_bytecode(WASMModuleInstance *module_inst,
exec_env, module_inst->func_ptrs[func_idx], func_type, NULL, NULL,
argv, argc, argv);

if (clear_wasi_proc_exit_exception(module_inst))
ret = true;

return ret && !wasm_get_exception(module_inst) ? true : false;
}
}
Expand Down
22 changes: 1 addition & 21 deletions core/iwasm/interpreter/wasm_runtime.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2033,24 +2033,6 @@ wasm_lookup_table(const WASMModuleInstance *module_inst, const char *name)
}
#endif

static bool
clear_wasi_proc_exit_exception(WASMModuleInstance *module_inst)
{
#if WASM_ENABLE_LIBC_WASI != 0
const char *exception = wasm_get_exception(module_inst);
if (exception && !strcmp(exception, "Exception: wasi proc exit")) {
/* The "wasi proc exit" exception is thrown by native lib to
let wasm app exit, which is a normal behavior, we clear
the exception here. */
wasm_set_exception(module_inst, NULL);
return true;
}
return false;
#else
return false;
#endif
}

#ifdef OS_ENABLE_HW_BOUND_CHECK

static void
Expand Down Expand Up @@ -2160,7 +2142,6 @@ wasm_call_function(WASMExecEnv *exec_env, WASMFunctionInstance *function,
wasm_exec_env_set_thread_info(exec_env);

interp_call_wasm(module_inst, exec_env, function, argc, argv);
(void)clear_wasi_proc_exit_exception(module_inst);
return !wasm_get_exception(module_inst) ? true : false;
}

Expand Down Expand Up @@ -2188,7 +2169,7 @@ wasm_create_exec_env_and_call_function(WASMModuleInstance *module_inst,
}
}

ret = wasm_call_function(exec_env, func, argc, argv);
ret = wasm_runtime_call_wasm(exec_env, func, argc, argv);

/* don't destroy the exec_env if it isn't created in this function */
if (!existing_exec_env)
Expand Down Expand Up @@ -2458,7 +2439,6 @@ call_indirect(WASMExecEnv *exec_env, uint32 tbl_idx, uint32 elem_idx,

interp_call_wasm(module_inst, exec_env, func_inst, argc, argv);

(void)clear_wasi_proc_exit_exception(module_inst);
return !wasm_get_exception(module_inst) ? true : false;

got_exception:
Expand Down
3 changes: 1 addition & 2 deletions core/iwasm/libraries/lib-pthread/lib_pthread_wrapper.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -520,8 +520,7 @@ pthread_start_routine(void *arg)

if (!wasm_runtime_call_indirect(exec_env, routine_args->elem_index, 1,
argv)) {
if (wasm_runtime_get_exception(module_inst))
wasm_cluster_spread_exception(exec_env);
/* Exception has already been spread during throwing */
}

/* destroy pthread key values */
Expand Down
Loading