From 9f68c4f1f5c5948b5646264847fec4cd6affe2c0 Mon Sep 17 00:00:00 2001 From: eloparco Date: Mon, 20 Feb 2023 13:52:29 +0000 Subject: [PATCH 01/10] fix(wasi-threads): return exit code set by proc_exit --- core/iwasm/interpreter/wasm_interp_classic.c | 12 ++++++++++-- core/iwasm/interpreter/wasm_interp_fast.c | 11 +++++++++++ core/iwasm/libraries/thread-mgr/thread_manager.c | 2 ++ 3 files changed, 23 insertions(+), 2 deletions(-) diff --git a/core/iwasm/interpreter/wasm_interp_classic.c b/core/iwasm/interpreter/wasm_interp_classic.c index 7b193f63c8..a9c4ef5f6f 100644 --- a/core/iwasm/interpreter/wasm_interp_classic.c +++ b/core/iwasm/interpreter/wasm_interp_classic.c @@ -3419,6 +3419,10 @@ wasm_interp_call_func_bytecode(WASMModuleInstance *module, ret = wasm_runtime_atomic_wait( (WASMModuleInstanceCommon *)module, maddr, (uint64)expect, timeout, false); + +#if WASM_ENABLE_THREAD_MGR != 0 + CHECK_SUSPEND_FLAGS(); +#endif if (ret == (uint32)-1) goto got_exception; @@ -3439,6 +3443,10 @@ wasm_interp_call_func_bytecode(WASMModuleInstance *module, ret = wasm_runtime_atomic_wait( (WASMModuleInstanceCommon *)module, maddr, expect, timeout, true); + +#if WASM_ENABLE_THREAD_MGR != 0 + CHECK_SUSPEND_FLAGS(); +#endif if (ret == (uint32)-1) goto got_exception; @@ -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..efca4fb67d 100644 --- a/core/iwasm/interpreter/wasm_interp_fast.c +++ b/core/iwasm/interpreter/wasm_interp_fast.c @@ -3263,6 +3263,10 @@ wasm_interp_call_func_bytecode(WASMModuleInstance *module, ret = wasm_runtime_atomic_wait( (WASMModuleInstanceCommon *)module, maddr, (uint64)expect, timeout, false); + +#if WASM_ENABLE_THREAD_MGR != 0 + CHECK_SUSPEND_FLAGS(); +#endif if (ret == (uint32)-1) goto got_exception; @@ -3283,6 +3287,10 @@ wasm_interp_call_func_bytecode(WASMModuleInstance *module, ret = wasm_runtime_atomic_wait( (WASMModuleInstanceCommon *)module, maddr, expect, timeout, true); + +#if WASM_ENABLE_THREAD_MGR != 0 + CHECK_SUSPEND_FLAGS(); +#endif if (ret == (uint32)-1) goto got_exception; @@ -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..eba8cec104 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 */ From f8376517d193bfea7fa6494cf99fc4d3caff6dde Mon Sep 17 00:00:00 2001 From: eloparco Date: Wed, 22 Feb 2023 16:24:54 +0000 Subject: [PATCH 02/10] fix: add suspend flags check in AOT mode --- core/iwasm/compilation/aot_emit_memory.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/core/iwasm/compilation/aot_emit_memory.c b/core/iwasm/compilation/aot_emit_memory.c index 7224d9f5df..cf32e1b23d 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,6 +1345,14 @@ 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(LLVMIntSGT, ret_value, I32_ZERO, cmp, "atomic_wait_ret"); ADD_BASIC_BLOCK(wait_fail, "atomic_wait_fail"); From 9c51c31c7043386e005ba7f94a0b39ebf363c53f Mon Sep 17 00:00:00 2001 From: eloparco Date: Thu, 23 Feb 2023 02:27:57 +0000 Subject: [PATCH 03/10] fix: fix atomic wait return code comparison --- core/iwasm/compilation/aot_emit_memory.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/iwasm/compilation/aot_emit_memory.c b/core/iwasm/compilation/aot_emit_memory.c index cf32e1b23d..a0934c2cd0 100644 --- a/core/iwasm/compilation/aot_emit_memory.c +++ b/core/iwasm/compilation/aot_emit_memory.c @@ -1353,7 +1353,7 @@ aot_compile_op_atomic_wait(AOTCompContext *comp_ctx, AOTFuncContext *func_ctx, } #endif - BUILD_ICMP(LLVMIntSGT, ret_value, I32_ZERO, cmp, "atomic_wait_ret"); + 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"); From fcb1f0ca6d9f2bb3bea31d724a853c6c9a2cfdf6 Mon Sep 17 00:00:00 2001 From: eloparco Date: Thu, 23 Feb 2023 10:39:15 +0000 Subject: [PATCH 04/10] doc: update wasi-threads readme --- samples/wasi-threads/README.md | 1 + 1 file changed, 1 insertion(+) 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 ``` From 56ee840ee6898b7090f8b8417634b1712892b890 Mon Sep 17 00:00:00 2001 From: eloparco Date: Thu, 23 Feb 2023 11:18:54 +0000 Subject: [PATCH 05/10] fix: add check for thread termination after function call in AOT mode --- core/iwasm/compilation/aot_emit_function.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/core/iwasm/compilation/aot_emit_function.c b/core/iwasm/compilation/aot_emit_function.c index 4ac62a9eaf..4742cdb8d4 100644 --- a/core/iwasm/compilation/aot_emit_function.c +++ b/core/iwasm/compilation/aot_emit_function.c @@ -857,6 +857,14 @@ aot_compile_op_call(AOTCompContext *comp_ctx, AOTFuncContext *func_ctx, goto fail; } +#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 + /* Check whether there was exception thrown when executing the function */ if (!check_exception_thrown(comp_ctx, func_ctx)) { From 7d5a80889e41d11b0fcb0c36da80f4d979ac82dc Mon Sep 17 00:00:00 2001 From: eloparco Date: Thu, 23 Feb 2023 11:20:58 +0000 Subject: [PATCH 06/10] fix: remove assert in signal handler --- core/iwasm/common/wasm_runtime_common.c | 1 - 1 file changed, 1 deletion(-) diff --git a/core/iwasm/common/wasm_runtime_common.c b/core/iwasm/common/wasm_runtime_common.c index 4aa3d7ed89..7c69b5a21f 100644 --- a/core/iwasm/common/wasm_runtime_common.c +++ b/core/iwasm/common/wasm_runtime_common.c @@ -194,7 +194,6 @@ runtime_signal_handler(void *sig_addr) else if (exec_env_tls->exce_check_guard_page <= (uint8 *)sig_addr && (uint8 *)sig_addr < exec_env_tls->exce_check_guard_page + page_size) { - bh_assert(wasm_get_exception(module_inst)); os_longjmp(jmpbuf_node->jmpbuf, 1); } } From 619271934ef052c3aea496e23c1d874cf2551952 Mon Sep 17 00:00:00 2001 From: eloparco Date: Thu, 23 Feb 2023 15:18:47 +0000 Subject: [PATCH 07/10] fix: update assert condition and move termination check --- core/iwasm/common/wasm_runtime_common.c | 11 ++++++++- core/iwasm/compilation/aot_emit_function.c | 24 ++++++++++++------- .../libraries/thread-mgr/thread_manager.c | 5 ++-- 3 files changed, 29 insertions(+), 11 deletions(-) diff --git a/core/iwasm/common/wasm_runtime_common.c b/core/iwasm/common/wasm_runtime_common.c index 7c69b5a21f..27ae3527a8 100644 --- a/core/iwasm/common/wasm_runtime_common.c +++ b/core/iwasm/common/wasm_runtime_common.c @@ -194,6 +194,11 @@ runtime_signal_handler(void *sig_addr) else if (exec_env_tls->exce_check_guard_page <= (uint8 *)sig_addr && (uint8 *)sig_addr < exec_env_tls->exce_check_guard_page + page_size) { + bh_assert(wasm_get_exception(module_inst) +#if WASM_ENABLE_THREAD_MGR != 0 + || wasm_cluster_is_thread_terminated(exec_env_tls) +#endif + ); os_longjmp(jmpbuf_node->jmpbuf, 1); } } @@ -249,7 +254,11 @@ runtime_exception_handler(EXCEPTION_POINTERS *exce_info) else if (exec_env_tls->exce_check_guard_page <= (uint8 *)sig_addr && (uint8 *)sig_addr < exec_env_tls->exce_check_guard_page + page_size) { - bh_assert(wasm_get_exception(module_inst)); + bh_assert(wasm_get_exception(module_inst) +#if WASM_ENABLE_THREAD_MGR != 0 + || wasm_cluster_is_thread_terminated(exec_env_tls) +#endif + ); if (module_inst->module_type == Wasm_Module_Bytecode) { return EXCEPTION_CONTINUE_SEARCH; } diff --git a/core/iwasm/compilation/aot_emit_function.c b/core/iwasm/compilation/aot_emit_function.c index 4742cdb8d4..9b977fc24a 100644 --- a/core/iwasm/compilation/aot_emit_function.c +++ b/core/iwasm/compilation/aot_emit_function.c @@ -857,14 +857,6 @@ aot_compile_op_call(AOTCompContext *comp_ctx, AOTFuncContext *func_ctx, goto fail; } -#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 - /* Check whether there was exception thrown when executing the function */ if (!check_exception_thrown(comp_ctx, func_ctx)) { @@ -1007,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)) + return false; + } +#endif + ret = true; fail: if (param_types) @@ -1653,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)) + return false; + } +#endif + ret = true; fail: diff --git a/core/iwasm/libraries/thread-mgr/thread_manager.c b/core/iwasm/libraries/thread-mgr/thread_manager.c index eba8cec104..3b1a490995 100644 --- a/core/iwasm/libraries/thread-mgr/thread_manager.c +++ b/core/iwasm/libraries/thread-mgr/thread_manager.c @@ -1136,9 +1136,10 @@ set_exception_visitor(void *node, void *user_data) 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); } + + /* Terminate the thread so it can exit from dead loops */ + set_thread_cancel_flags(curr_exec_env); } static void From 1f0494475f07d66ada8c26c0c736c2ebb0c65daf Mon Sep 17 00:00:00 2001 From: eloparco Date: Fri, 24 Feb 2023 00:36:06 +0000 Subject: [PATCH 08/10] fix: revert changes in previous commit --- core/iwasm/common/wasm_runtime_common.c | 6 ++---- core/iwasm/libraries/thread-mgr/thread_manager.c | 6 +++--- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/core/iwasm/common/wasm_runtime_common.c b/core/iwasm/common/wasm_runtime_common.c index 27ae3527a8..a7b2b9f9f5 100644 --- a/core/iwasm/common/wasm_runtime_common.c +++ b/core/iwasm/common/wasm_runtime_common.c @@ -194,11 +194,9 @@ runtime_signal_handler(void *sig_addr) else if (exec_env_tls->exce_check_guard_page <= (uint8 *)sig_addr && (uint8 *)sig_addr < exec_env_tls->exce_check_guard_page + page_size) { - bh_assert(wasm_get_exception(module_inst) -#if WASM_ENABLE_THREAD_MGR != 0 - || wasm_cluster_is_thread_terminated(exec_env_tls) +#if WASM_ENABLE_THREAD_MGR == 0 + bh_assert(wasm_get_exception(module_inst)); #endif - ); os_longjmp(jmpbuf_node->jmpbuf, 1); } } diff --git a/core/iwasm/libraries/thread-mgr/thread_manager.c b/core/iwasm/libraries/thread-mgr/thread_manager.c index 3b1a490995..75b29fdb3c 100644 --- a/core/iwasm/libraries/thread-mgr/thread_manager.c +++ b/core/iwasm/libraries/thread-mgr/thread_manager.c @@ -1136,10 +1136,10 @@ set_exception_visitor(void *node, void *user_data) 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); + /* Terminate the thread so it can exit from dead loops */ + set_thread_cancel_flags(curr_exec_env); + } } static void From e4075dc57db1054bbe4ba327f21ca51236429887 Mon Sep 17 00:00:00 2001 From: eloparco Date: Fri, 24 Feb 2023 01:07:13 +0000 Subject: [PATCH 09/10] fix: add option to use atomic wait and poll_oneoff in thread termination sample --- .../wasm-apps/thread_termination.c | 29 +++++++++++++------ 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/samples/wasi-threads/wasm-apps/thread_termination.c b/samples/wasi-threads/wasm-apps/thread_termination.c index dfc228eb3c..9a4de87d5d 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) { From fcc45c885e4d25cbc159f796c428667ba954066b Mon Sep 17 00:00:00 2001 From: eloparco Date: Fri, 24 Feb 2023 01:15:03 +0000 Subject: [PATCH 10/10] fix: clear resources when thread terminated in AOT mode --- core/iwasm/compilation/aot_emit_function.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/iwasm/compilation/aot_emit_function.c b/core/iwasm/compilation/aot_emit_function.c index 9b977fc24a..9ba8baa24f 100644 --- a/core/iwasm/compilation/aot_emit_function.c +++ b/core/iwasm/compilation/aot_emit_function.c @@ -1003,7 +1003,7 @@ aot_compile_op_call(AOTCompContext *comp_ctx, AOTFuncContext *func_ctx, /* Insert suspend check point */ if (comp_ctx->enable_thread_mgr) { if (!check_suspend_flags(comp_ctx, func_ctx)) - return false; + goto fail; } #endif @@ -1657,7 +1657,7 @@ aot_compile_op_call_indirect(AOTCompContext *comp_ctx, AOTFuncContext *func_ctx, /* Insert suspend check point */ if (comp_ctx->enable_thread_mgr) { if (!check_suspend_flags(comp_ctx, func_ctx)) - return false; + goto fail; } #endif