From b1e4e4f122954ee6260f3c85a6125d03b8988ea9 Mon Sep 17 00:00:00 2001 From: Wenyong Huang Date: Sat, 25 Feb 2023 07:17:42 +0800 Subject: [PATCH 1/3] thread-mgr: Fix spread "wasi proc exit" exception and atomic.wait issues --- core/iwasm/common/wasm_runtime_common.c | 2 +- core/iwasm/compilation/aot_emit_memory.c | 11 +++++- core/iwasm/interpreter/wasm_interp_classic.c | 12 +++++- core/iwasm/interpreter/wasm_interp_fast.c | 11 ++++++ .../libraries/thread-mgr/thread_manager.c | 37 +++++++------------ .../libraries/thread-mgr/thread_manager.h | 2 +- samples/wasi-threads/README.md | 1 + .../wasm-apps/thread_termination.c | 31 +++++++++++----- 8 files changed, 69 insertions(+), 38 deletions(-) diff --git a/core/iwasm/common/wasm_runtime_common.c b/core/iwasm/common/wasm_runtime_common.c index 4aa3d7ed89..a7b37f55c5 100644 --- a/core/iwasm/common/wasm_runtime_common.c +++ b/core/iwasm/common/wasm_runtime_common.c @@ -2316,7 +2316,7 @@ wasm_set_exception(WASMModuleInstance *module_inst, const char *exception) exec_env = wasm_clusters_search_exec_env((WASMModuleInstanceCommon *)module_inst); if (exec_env) { - wasm_cluster_spread_exception(exec_env, exception ? false : true); + wasm_cluster_spread_exception(exec_env); } #if WASM_ENABLE_SHARED_MEMORY if (exception) { diff --git a/core/iwasm/compilation/aot_emit_memory.c b/core/iwasm/compilation/aot_emit_memory.c index 7224d9f5df..a0934c2cd0 100644 --- a/core/iwasm/compilation/aot_emit_memory.c +++ b/core/iwasm/compilation/aot_emit_memory.c @@ -7,6 +7,7 @@ #include "aot_emit_exception.h" #include "../aot/aot_runtime.h" #include "aot_intrinsic.h" +#include "aot_emit_control.h" #define BUILD_ICMP(op, left, right, res, name) \ do { \ @@ -1344,7 +1345,15 @@ aot_compile_op_atomic_wait(AOTCompContext *comp_ctx, AOTFuncContext *func_ctx, return false; } - BUILD_ICMP(LLVMIntSGT, ret_value, I32_ZERO, cmp, "atomic_wait_ret"); +#if WASM_ENABLE_THREAD_MGR != 0 + /* Insert suspend check point */ + if (comp_ctx->enable_thread_mgr) { + if (!check_suspend_flags(comp_ctx, func_ctx)) + return false; + } +#endif + + BUILD_ICMP(LLVMIntNE, ret_value, I32_NEG_ONE, cmp, "atomic_wait_ret"); ADD_BASIC_BLOCK(wait_fail, "atomic_wait_fail"); ADD_BASIC_BLOCK(wait_success, "wait_success"); diff --git a/core/iwasm/interpreter/wasm_interp_classic.c b/core/iwasm/interpreter/wasm_interp_classic.c index 7b193f63c8..ac6763c697 100644 --- a/core/iwasm/interpreter/wasm_interp_classic.c +++ b/core/iwasm/interpreter/wasm_interp_classic.c @@ -3422,6 +3422,10 @@ wasm_interp_call_func_bytecode(WASMModuleInstance *module, if (ret == (uint32)-1) goto got_exception; +#if WASM_ENABLE_THREAD_MGR != 0 + CHECK_SUSPEND_FLAGS(); +#endif + PUSH_I32(ret); break; } @@ -3442,6 +3446,10 @@ wasm_interp_call_func_bytecode(WASMModuleInstance *module, if (ret == (uint32)-1) goto got_exception; +#if WASM_ENABLE_THREAD_MGR != 0 + CHECK_SUSPEND_FLAGS(); +#endif + PUSH_I32(ret); break; } @@ -3894,10 +3902,10 @@ wasm_interp_call_func_bytecode(WASMModuleInstance *module, PUSH_CSP(LABEL_TYPE_FUNCTION, 0, cell_num, frame_ip_end - 1); wasm_exec_env_set_cur_frame(exec_env, frame); + } #if WASM_ENABLE_THREAD_MGR != 0 - CHECK_SUSPEND_FLAGS(); + CHECK_SUSPEND_FLAGS(); #endif - } HANDLE_OP_END(); } diff --git a/core/iwasm/interpreter/wasm_interp_fast.c b/core/iwasm/interpreter/wasm_interp_fast.c index be924646af..7a8f474c5d 100644 --- a/core/iwasm/interpreter/wasm_interp_fast.c +++ b/core/iwasm/interpreter/wasm_interp_fast.c @@ -3266,6 +3266,10 @@ wasm_interp_call_func_bytecode(WASMModuleInstance *module, if (ret == (uint32)-1) goto got_exception; +#if WASM_ENABLE_THREAD_MGR != 0 + CHECK_SUSPEND_FLAGS(); +#endif + PUSH_I32(ret); break; } @@ -3286,6 +3290,10 @@ wasm_interp_call_func_bytecode(WASMModuleInstance *module, if (ret == (uint32)-1) goto got_exception; +#if WASM_ENABLE_THREAD_MGR != 0 + CHECK_SUSPEND_FLAGS(); +#endif + PUSH_I32(ret); break; } @@ -3826,6 +3834,9 @@ wasm_interp_call_func_bytecode(WASMModuleInstance *module, wasm_exec_env_set_cur_frame(exec_env, (WASMRuntimeFrame *)frame); } +#if WASM_ENABLE_THREAD_MGR != 0 + CHECK_SUSPEND_FLAGS(); +#endif HANDLE_OP_END(); } diff --git a/core/iwasm/libraries/thread-mgr/thread_manager.c b/core/iwasm/libraries/thread-mgr/thread_manager.c index 4d66beb6fd..1a1f9556ca 100644 --- a/core/iwasm/libraries/thread-mgr/thread_manager.c +++ b/core/iwasm/libraries/thread-mgr/thread_manager.c @@ -225,7 +225,9 @@ wasm_cluster_create(WASMExecEnv *exec_env) /* Prepare the aux stack top and size for every thread */ if (!wasm_exec_env_get_aux_stack(exec_env, &aux_stack_start, &aux_stack_size)) { +#if WASM_ENABLE_LIB_WASI_THREADS == 0 LOG_VERBOSE("No aux stack info for this module, can't create thread"); +#endif /* If the module don't have aux stack info, don't throw error here, but remain stack_tops and stack_segment_occupied as NULL */ @@ -1131,39 +1133,28 @@ set_exception_visitor(void *node, void *user_data) WASMModuleInstance *curr_wasm_inst = (WASMModuleInstance *)get_module_inst(curr_exec_env); - bh_memcpy_s(curr_wasm_inst->cur_exception, - sizeof(curr_wasm_inst->cur_exception), - wasm_inst->cur_exception, sizeof(wasm_inst->cur_exception)); + /* Only spread non "wasi proc exit" exception */ + if (!strstr(wasm_inst->cur_exception, "wasi proc exit")) { + bh_memcpy_s(curr_wasm_inst->cur_exception, + sizeof(curr_wasm_inst->cur_exception), + wasm_inst->cur_exception, + sizeof(wasm_inst->cur_exception)); + } + /* Terminate the thread so it can exit from dead loops */ set_thread_cancel_flags(curr_exec_env); } } -static void -clear_exception_visitor(void *node, void *user_data) -{ - WASMExecEnv *exec_env = (WASMExecEnv *)user_data; - WASMExecEnv *curr_exec_env = (WASMExecEnv *)node; - - if (curr_exec_env != exec_env) { - WASMModuleInstance *curr_wasm_inst = - (WASMModuleInstance *)get_module_inst(curr_exec_env); - - curr_wasm_inst->cur_exception[0] = '\0'; - } -} - void -wasm_cluster_spread_exception(WASMExecEnv *exec_env, bool clear) +wasm_cluster_spread_exception(WASMExecEnv *exec_env) { WASMCluster *cluster = wasm_exec_env_get_cluster(exec_env); bh_assert(cluster); os_mutex_lock(&cluster->lock); - cluster->has_exception = !clear; - traverse_list(&cluster->exec_env_list, - clear ? clear_exception_visitor : set_exception_visitor, - exec_env); + cluster->has_exception = true; + traverse_list(&cluster->exec_env_list, set_exception_visitor, exec_env); os_mutex_unlock(&cluster->lock); } @@ -1203,4 +1194,4 @@ bool wasm_cluster_is_thread_terminated(WASMExecEnv *exec_env) { return (exec_env->suspend_flags.flags & 0x01) ? true : false; -} \ No newline at end of file +} diff --git a/core/iwasm/libraries/thread-mgr/thread_manager.h b/core/iwasm/libraries/thread-mgr/thread_manager.h index 38ca167fb3..c9f1a4b962 100644 --- a/core/iwasm/libraries/thread-mgr/thread_manager.h +++ b/core/iwasm/libraries/thread-mgr/thread_manager.h @@ -134,7 +134,7 @@ WASMExecEnv * wasm_clusters_search_exec_env(WASMModuleInstanceCommon *module_inst); void -wasm_cluster_spread_exception(WASMExecEnv *exec_env, bool clear); +wasm_cluster_spread_exception(WASMExecEnv *exec_env); WASMExecEnv * wasm_cluster_spawn_exec_env(WASMExecEnv *exec_env); diff --git a/samples/wasi-threads/README.md b/samples/wasi-threads/README.md index 499162b6ec..ca8d166d2c 100644 --- a/samples/wasi-threads/README.md +++ b/samples/wasi-threads/README.md @@ -26,6 +26,7 @@ $ ./iwasm wasm-apps/exception_propagation.wasm ## Run samples in AOT mode ```shell $ ../../../wamr-compiler/build/wamrc \ + --enable-multi-thread \ -o wasm-apps/no_pthread.aot wasm-apps/no_pthread.wasm $ ./iwasm wasm-apps/no_pthread.aot ``` diff --git a/samples/wasi-threads/wasm-apps/thread_termination.c b/samples/wasi-threads/wasm-apps/thread_termination.c index dfc228eb3c..355bb3f453 100644 --- a/samples/wasi-threads/wasm-apps/thread_termination.c +++ b/samples/wasi-threads/wasm-apps/thread_termination.c @@ -15,8 +15,14 @@ #include "wasi_thread_start.h" -#define TEST_TERMINATION_BY_TRAP 0 // Otherwise test `proc_exit` termination -#define TEST_TERMINATION_IN_MAIN_THREAD 1 +#define BUSY_WAIT 0 +#define ATOMIC_WAIT 1 +#define POLL_ONEOFF 2 + +/* Change parameters here to modify the sample behavior */ +#define TEST_TERMINATION_BY_TRAP 0 /* Otherwise `proc_exit` termination */ +#define TEST_TERMINATION_IN_MAIN_THREAD 1 /* Otherwise in spawn thread */ +#define LONG_TASK_IMPL ATOMIC_WAIT #define TIMEOUT_SECONDS 10 #define NUM_THREADS 3 @@ -30,23 +36,28 @@ typedef struct { void run_long_task() { - // Busy waiting to be interruptible by trap or `proc_exit` +#if LONG_TASK_IMPL == BUSY_WAIT for (int i = 0; i < TIMEOUT_SECONDS; i++) sleep(1); +#elif LONG_TASK_IMPL == ATOMIC_WAIT + __builtin_wasm_memory_atomic_wait32(0, 0, -1); +#else + sleep(TIMEOUT_SECONDS); +#endif } void start_job() { sem_post(&sem); - run_long_task(); // Wait to be interrupted + run_long_task(); /* Wait to be interrupted */ assert(false && "Unreachable"); } void terminate_process() { - // Wait for all other threads (including main thread) to be ready + /* Wait for all other threads (including main thread) to be ready */ printf("Waiting before terminating\n"); for (int i = 0; i < NUM_THREADS; i++) sem_wait(&sem); @@ -55,7 +66,7 @@ terminate_process() #if TEST_TERMINATION_BY_TRAP == 1 __builtin_trap(); #else - __wasi_proc_exit(1); + __wasi_proc_exit(33); #endif } @@ -86,14 +97,14 @@ main(int argc, char **argv) } for (i = 0; i < NUM_THREADS; i++) { - // No graceful memory free to simplify the example + /* No graceful memory free to simplify the example */ if (!start_args_init(&data[i].base)) { printf("Failed to allocate thread's stack\n"); return EXIT_FAILURE; } } -// Create a thread that forces termination through trap or `proc_exit` + /* Create a thread that forces termination through trap or `proc_exit` */ #if TEST_TERMINATION_IN_MAIN_THREAD == 1 data[0].throw_exception = false; #else @@ -105,7 +116,7 @@ main(int argc, char **argv) return EXIT_FAILURE; } - // Create two additional threads to test exception propagation + /* Create two additional threads to test exception propagation */ data[1].throw_exception = false; thread_id = __wasi_thread_spawn(&data[1]); if (thread_id < 0) { @@ -128,4 +139,4 @@ main(int argc, char **argv) start_job(); #endif /* TEST_TERMINATION_IN_MAIN_THREAD */ return EXIT_SUCCESS; -} \ No newline at end of file +} From 2880a239c426a1229ff62a632924a4debcd5e462 Mon Sep 17 00:00:00 2001 From: Wenyong Huang Date: Sat, 25 Feb 2023 08:15:05 +0800 Subject: [PATCH 2/3] Restore cluster clear exception --- core/iwasm/common/wasm_runtime_common.c | 8 ++++--- core/iwasm/compilation/aot_emit_memory.c | 16 +++++++------- .../libraries/thread-mgr/thread_manager.c | 22 ++++++++++++++++--- .../libraries/thread-mgr/thread_manager.h | 2 +- 4 files changed, 33 insertions(+), 15 deletions(-) diff --git a/core/iwasm/common/wasm_runtime_common.c b/core/iwasm/common/wasm_runtime_common.c index a7b37f55c5..0b09a9db74 100644 --- a/core/iwasm/common/wasm_runtime_common.c +++ b/core/iwasm/common/wasm_runtime_common.c @@ -1880,8 +1880,10 @@ clear_wasi_proc_exit_exception(WASMModuleInstanceCommon *module_inst_comm) 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); + the exception here. And just clear the exception of current + thread, don't call `wasm_set_exception(module_inst, NULL)` + which will clear the exception of all threads. */ + module_inst->cur_exception[0] = '\0'; return true; } return false; @@ -2316,7 +2318,7 @@ wasm_set_exception(WASMModuleInstance *module_inst, const char *exception) exec_env = wasm_clusters_search_exec_env((WASMModuleInstanceCommon *)module_inst); if (exec_env) { - wasm_cluster_spread_exception(exec_env); + wasm_cluster_spread_exception(exec_env, exception ? false : true); } #if WASM_ENABLE_SHARED_MEMORY if (exception) { diff --git a/core/iwasm/compilation/aot_emit_memory.c b/core/iwasm/compilation/aot_emit_memory.c index a0934c2cd0..26d9f92295 100644 --- a/core/iwasm/compilation/aot_emit_memory.c +++ b/core/iwasm/compilation/aot_emit_memory.c @@ -1345,14 +1345,6 @@ aot_compile_op_atomic_wait(AOTCompContext *comp_ctx, AOTFuncContext *func_ctx, return false; } -#if WASM_ENABLE_THREAD_MGR != 0 - /* Insert suspend check point */ - if (comp_ctx->enable_thread_mgr) { - if (!check_suspend_flags(comp_ctx, func_ctx)) - return false; - } -#endif - BUILD_ICMP(LLVMIntNE, ret_value, I32_NEG_ONE, cmp, "atomic_wait_ret"); ADD_BASIC_BLOCK(wait_fail, "atomic_wait_fail"); @@ -1377,6 +1369,14 @@ aot_compile_op_atomic_wait(AOTCompContext *comp_ctx, AOTFuncContext *func_ctx, PUSH_I32(ret_value); +#if WASM_ENABLE_THREAD_MGR != 0 + /* Insert suspend check point */ + if (comp_ctx->enable_thread_mgr) { + if (!check_suspend_flags(comp_ctx, func_ctx)) + return false; + } +#endif + return true; fail: return false; diff --git a/core/iwasm/libraries/thread-mgr/thread_manager.c b/core/iwasm/libraries/thread-mgr/thread_manager.c index 1a1f9556ca..79048c04a7 100644 --- a/core/iwasm/libraries/thread-mgr/thread_manager.c +++ b/core/iwasm/libraries/thread-mgr/thread_manager.c @@ -1146,15 +1146,31 @@ set_exception_visitor(void *node, void *user_data) } } +static void +clear_exception_visitor(void *node, void *user_data) +{ + WASMExecEnv *exec_env = (WASMExecEnv *)user_data; + WASMExecEnv *curr_exec_env = (WASMExecEnv *)node; + + if (curr_exec_env != exec_env) { + WASMModuleInstance *curr_wasm_inst = + (WASMModuleInstance *)get_module_inst(curr_exec_env); + + curr_wasm_inst->cur_exception[0] = '\0'; + } +} + void -wasm_cluster_spread_exception(WASMExecEnv *exec_env) +wasm_cluster_spread_exception(WASMExecEnv *exec_env, bool clear) { WASMCluster *cluster = wasm_exec_env_get_cluster(exec_env); bh_assert(cluster); os_mutex_lock(&cluster->lock); - cluster->has_exception = true; - traverse_list(&cluster->exec_env_list, set_exception_visitor, exec_env); + cluster->has_exception = !clear; + traverse_list(&cluster->exec_env_list, + clear ? clear_exception_visitor : set_exception_visitor, + exec_env); os_mutex_unlock(&cluster->lock); } diff --git a/core/iwasm/libraries/thread-mgr/thread_manager.h b/core/iwasm/libraries/thread-mgr/thread_manager.h index c9f1a4b962..38ca167fb3 100644 --- a/core/iwasm/libraries/thread-mgr/thread_manager.h +++ b/core/iwasm/libraries/thread-mgr/thread_manager.h @@ -134,7 +134,7 @@ WASMExecEnv * wasm_clusters_search_exec_env(WASMModuleInstanceCommon *module_inst); void -wasm_cluster_spread_exception(WASMExecEnv *exec_env); +wasm_cluster_spread_exception(WASMExecEnv *exec_env, bool clear); WASMExecEnv * wasm_cluster_spawn_exec_env(WASMExecEnv *exec_env); From 99ca90f256f89776664f4c31992306eb929abfdc Mon Sep 17 00:00:00 2001 From: Wenyong Huang Date: Sat, 25 Feb 2023 09:08:06 +0800 Subject: [PATCH 3/3] Add check suspend flags in AOT after calling function --- core/iwasm/compilation/aot_emit_function.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/core/iwasm/compilation/aot_emit_function.c b/core/iwasm/compilation/aot_emit_function.c index 4ac62a9eaf..9ba8baa24f 100644 --- a/core/iwasm/compilation/aot_emit_function.c +++ b/core/iwasm/compilation/aot_emit_function.c @@ -999,6 +999,14 @@ aot_compile_op_call(AOTCompContext *comp_ctx, AOTFuncContext *func_ctx, } #endif +#if WASM_ENABLE_THREAD_MGR != 0 + /* Insert suspend check point */ + if (comp_ctx->enable_thread_mgr) { + if (!check_suspend_flags(comp_ctx, func_ctx)) + goto fail; + } +#endif + ret = true; fail: if (param_types) @@ -1645,6 +1653,14 @@ aot_compile_op_call_indirect(AOTCompContext *comp_ctx, AOTFuncContext *func_ctx, } #endif +#if WASM_ENABLE_THREAD_MGR != 0 + /* Insert suspend check point */ + if (comp_ctx->enable_thread_mgr) { + if (!check_suspend_flags(comp_ctx, func_ctx)) + goto fail; + } +#endif + ret = true; fail: