From 0f0ea837e514ab3da22aa1197edc580d8172b63a Mon Sep 17 00:00:00 2001 From: eloparco Date: Thu, 26 Jan 2023 11:30:12 +0000 Subject: [PATCH 1/7] feat(wasi-threads) interrupt blocking instructions using signals --- core/iwasm/interpreter/wasm_interp_classic.c | 63 +++++++++++++++++-- .../libraries/thread-mgr/thread_manager.c | 4 ++ .../platform/common/posix/posix_thread.c | 33 ++++++++++ .../platform/include/platform_api_extension.h | 6 ++ samples/wasi-threads/wasm-apps/no_pthread.c | 28 ++++----- 5 files changed, 113 insertions(+), 21 deletions(-) diff --git a/core/iwasm/interpreter/wasm_interp_classic.c b/core/iwasm/interpreter/wasm_interp_classic.c index 06d75a4897..cf8734fa4e 100644 --- a/core/iwasm/interpreter/wasm_interp_classic.c +++ b/core/iwasm/interpreter/wasm_interp_classic.c @@ -99,6 +99,18 @@ typedef float64 CellType_F64; #define CHECK_WRITE_WATCHPOINT(addr, offset) (void)0 #endif +static uint32 +thread_handle_hash(void *handle) +{ + return (uint32)(uintptr_t)handle; +} + +static bool +thread_handle_equal(void *h1, void *h2) +{ + return (uint32)(uintptr_t)h1 == (uint32)(uintptr_t)h2 ? true : false; +} + static inline uint32 rotl32(uint32 n, uint32 c) { @@ -4183,6 +4195,40 @@ wasm_interp_call_wasm(WASMModuleInstance *module_inst, WASMExecEnv *exec_env, wasm_exec_env_set_cur_frame(exec_env, frame); + // Create map + // NOTE: The list is needed for nested calls to `wasm_interp_call_wasm` + if (contexts == NULL) { + contexts = bh_hash_map_create(32, false, (HashFunc)thread_handle_hash, + (KeyEqualFunc)thread_handle_equal, NULL, + wasm_runtime_free); + } + + // Get or create list of contexts for current thread + bh_list *context_list = + bh_hash_map_find(contexts, (void *)(uintptr_t)exec_env->handle); + if (!context_list) { + context_list = malloc(sizeof(bh_list)); + int ret = bh_list_init(context_list); + assert(ret == BH_LIST_SUCCESS); + + bool bh_ret = bh_hash_map_insert( + contexts, (void *)(uintptr_t)exec_env->handle, context_list); + assert(bh_ret); + } + assert(context_list); + + // Create context + sigjmp_buf *context = malloc(sizeof(sigjmp_buf)); + pthread_t self = pthread_self(); + printf("context=%p %ld\n", context, self); + + if (sigsetjmp(*context, 1) == 0) { + // Normal execution, i.e. not returning from sig handler + + // Add context to context list + int bh_res = bh_list_insert(context_list, context); + assert(bh_res == BH_LIST_SUCCESS); + if (function->is_import_func) { #if WASM_ENABLE_MULTI_MODULE != 0 if (function->import_module_inst) { @@ -4207,14 +4253,15 @@ wasm_interp_call_wasm(WASMModuleInstance *module_inst, WASMExecEnv *exec_env, uint32 func_idx = (uint32)(function - module_inst->e->functions); if (module_inst->module->func_ptrs_compiled [func_idx - module_inst->module->import_function_count]) { - llvm_jit_call_func_bytecode(module_inst, exec_env, function, argc, - argv); + llvm_jit_call_func_bytecode(module_inst, exec_env, function, + argc, argv); /* For llvm jit, the results have been stored in argv, no need to copy them from stack frame again */ copy_argv_from_frame = false; } else { - fast_jit_call_func_bytecode(module_inst, exec_env, function, frame); + fast_jit_call_func_bytecode(module_inst, exec_env, function, + frame); } #elif WASM_ENABLE_JIT != 0 /* Only LLVM JIT is enabled */ @@ -4228,7 +4275,8 @@ wasm_interp_call_wasm(WASMModuleInstance *module_inst, WASMExecEnv *exec_env, fast_jit_call_func_bytecode(module_inst, exec_env, function, frame); #else /* Both Fast JIT and LLVM JIT are disabled */ - wasm_interp_call_func_bytecode(module_inst, exec_env, function, frame); + wasm_interp_call_func_bytecode(module_inst, exec_env, function, + frame); #endif #else /* else of WASM_ENABLE_LAZY_JIT != 0 */ @@ -4246,7 +4294,8 @@ wasm_interp_call_wasm(WASMModuleInstance *module_inst, WASMExecEnv *exec_env, fast_jit_call_func_bytecode(module_inst, exec_env, function, frame); #else /* Both Fast JIT and LLVM JIT are disabled */ - wasm_interp_call_func_bytecode(module_inst, exec_env, function, frame); + wasm_interp_call_func_bytecode(module_inst, exec_env, function, + frame); #endif #endif /* end of WASM_ENABLE_LAZY_JIT != 0 */ @@ -4255,8 +4304,12 @@ wasm_interp_call_wasm(WASMModuleInstance *module_inst, WASMExecEnv *exec_env, #if WASM_ENABLE_FAST_JIT != 0 (void)fast_jit_call_func_bytecode; #endif + } } + printf("exit context=%p %p\n", context, (void *)self); + int bh_res = bh_list_remove(context_list, context); + assert(bh_res == BH_LIST_SUCCESS); /* Output the return value to the caller */ if (!wasm_get_exception(module_inst)) { if (copy_argv_from_frame) { diff --git a/core/iwasm/libraries/thread-mgr/thread_manager.c b/core/iwasm/libraries/thread-mgr/thread_manager.c index 492ed66b10..013aa075c8 100644 --- a/core/iwasm/libraries/thread-mgr/thread_manager.c +++ b/core/iwasm/libraries/thread-mgr/thread_manager.c @@ -1059,6 +1059,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)); + + if (curr_exec_env->handle) + os_thread_signal(curr_exec_env->handle, SIGTERM); + /* Terminate the thread so it can exit from dead loops */ set_thread_cancel_flags(curr_exec_env); } diff --git a/core/shared/platform/common/posix/posix_thread.c b/core/shared/platform/common/posix/posix_thread.c index 58370c203f..3e683f3eb8 100644 --- a/core/shared/platform/common/posix/posix_thread.c +++ b/core/shared/platform/common/posix/posix_thread.c @@ -9,6 +9,11 @@ #include "platform_api_vmcore.h" #include "platform_api_extension.h" +#include "bh_hashmap.h" +#include + +void *contexts = NULL; + typedef struct { thread_start_routine_t start; void *arg; @@ -47,6 +52,20 @@ os_thread_wrapper(void *arg) return NULL; } +static void +handler(int sig) +{ + korp_tid self = pthread_self(); + bh_list *context_list = bh_hash_map_find(contexts, (void *)(uintptr_t)self); + assert(context_list); + // Get latest context (first in the list) for current thread + sigjmp_buf *context = bh_list_first_elem(context_list); + assert(context); + + printf("handler, %p %ld\n", context, self); + siglongjmp(*context, 1); +} + int os_thread_create_with_prio(korp_tid *tid, thread_start_routine_t start, void *arg, unsigned int stack_size, int prio) @@ -54,6 +73,14 @@ os_thread_create_with_prio(korp_tid *tid, thread_start_routine_t start, pthread_attr_t tattr; thread_wrapper_arg *targ; + struct sigaction act; + memset(&act, 0, sizeof(act)); + act.sa_handler = handler; + act.sa_flags = SA_RESETHAND; + sigfillset(&act.sa_mask); + if (sigaction(SIGTERM, &act, NULL) < 0) + return BHT_ERROR; + assert(stack_size > 0); assert(tid); assert(start); @@ -89,6 +116,12 @@ os_thread_create_with_prio(korp_tid *tid, thread_start_routine_t start, return BHT_OK; } +int +os_thread_signal(korp_tid tid, int sig) +{ + return pthread_kill(tid, sig); +} + int os_thread_create(korp_tid *tid, thread_start_routine_t start, void *arg, unsigned int stack_size) diff --git a/core/shared/platform/include/platform_api_extension.h b/core/shared/platform/include/platform_api_extension.h index 42baad74f7..0a40801d55 100644 --- a/core/shared/platform/include/platform_api_extension.h +++ b/core/shared/platform/include/platform_api_extension.h @@ -17,6 +17,9 @@ extern "C" { #endif +#include +extern void *contexts; + /*************************************************** * * * Extension interface * @@ -49,6 +52,9 @@ int os_thread_create(korp_tid *p_tid, thread_start_routine_t start, void *arg, unsigned int stack_size); +int +os_thread_signal(korp_tid p_tid, int sig); + /** * Creates a thread with priority * diff --git a/samples/wasi-threads/wasm-apps/no_pthread.c b/samples/wasi-threads/wasm-apps/no_pthread.c index fcf64a7b76..277013a0a3 100644 --- a/samples/wasi-threads/wasm-apps/no_pthread.c +++ b/samples/wasi-threads/wasm-apps/no_pthread.c @@ -12,6 +12,8 @@ #include "wasi_thread_start.h" +#define STOP_FROM_MAIN 1 + static const int64_t SECOND = 1000 * 1000 * 1000; typedef struct { @@ -29,12 +31,11 @@ __wasi_thread_start_C(int thread_id, int *start_arg) printf("New thread ID: %d, starting parameter: %d\n", thread_id, data->value); - data->thread_id = thread_id; - data->value += 8; - printf("Updated value: %d\n", data->value); - - data->th_ready = 1; - __builtin_wasm_memory_atomic_notify(&data->th_ready, 1); +#if STOP_FROM_MAIN == 1 + __builtin_wasm_memory_atomic_wait32(0, 0, -1); +#else + __wasi_proc_exit(55); +#endif } int @@ -56,16 +57,11 @@ main(int argc, char **argv) goto final; } - if (__builtin_wasm_memory_atomic_wait32(&data.th_ready, 0, SECOND) == 2) { - printf("Timeout\n"); - ret = EXIT_FAILURE; - goto final; - } - - printf("Thread completed, new value: %d, thread id: %d\n", data.value, - data.thread_id); - - assert(thread_id == data.thread_id); +#if STOP_FROM_MAIN == 1 + __wasi_proc_exit(56); +#else + __builtin_wasm_memory_atomic_wait32(0, 0, -1); +#endif final: start_args_deinit(&data.base); From 8c940587d86ed17c0ce3481fb630a56b4137ba70 Mon Sep 17 00:00:00 2001 From: eloparco Date: Fri, 27 Jan 2023 09:54:26 +0000 Subject: [PATCH 2/7] feat(wasi-threads): move stack save/restore operations into platform specific code --- core/iwasm/interpreter/wasm_interp_classic.c | 142 ++++++------------ .../platform/common/posix/posix_thread.c | 73 +++++++++ .../platform/include/platform_api_extension.h | 10 ++ 3 files changed, 133 insertions(+), 92 deletions(-) diff --git a/core/iwasm/interpreter/wasm_interp_classic.c b/core/iwasm/interpreter/wasm_interp_classic.c index cf8734fa4e..2722e6dc73 100644 --- a/core/iwasm/interpreter/wasm_interp_classic.c +++ b/core/iwasm/interpreter/wasm_interp_classic.c @@ -99,18 +99,6 @@ typedef float64 CellType_F64; #define CHECK_WRITE_WATCHPOINT(addr, offset) (void)0 #endif -static uint32 -thread_handle_hash(void *handle) -{ - return (uint32)(uintptr_t)handle; -} - -static bool -thread_handle_equal(void *h1, void *h2) -{ - return (uint32)(uintptr_t)h1 == (uint32)(uintptr_t)h2 ? true : false; -} - static inline uint32 rotl32(uint32 n, uint32 c) { @@ -4195,121 +4183,91 @@ wasm_interp_call_wasm(WASMModuleInstance *module_inst, WASMExecEnv *exec_env, wasm_exec_env_set_cur_frame(exec_env, frame); - // Create map - // NOTE: The list is needed for nested calls to `wasm_interp_call_wasm` - if (contexts == NULL) { - contexts = bh_hash_map_create(32, false, (HashFunc)thread_handle_hash, - (KeyEqualFunc)thread_handle_equal, NULL, - wasm_runtime_free); - } - - // Get or create list of contexts for current thread - bh_list *context_list = - bh_hash_map_find(contexts, (void *)(uintptr_t)exec_env->handle); - if (!context_list) { - context_list = malloc(sizeof(bh_list)); - int ret = bh_list_init(context_list); - assert(ret == BH_LIST_SUCCESS); - - bool bh_ret = bh_hash_map_insert( - contexts, (void *)(uintptr_t)exec_env->handle, context_list); - assert(bh_ret); - } - assert(context_list); + sigjmp_buf *context = os_save_context(exec_env->handle); + if (!os_is_exception(exec_env->handle, context)) { - // Create context - sigjmp_buf *context = malloc(sizeof(sigjmp_buf)); - pthread_t self = pthread_self(); - printf("context=%p %ld\n", context, self); - - if (sigsetjmp(*context, 1) == 0) { - // Normal execution, i.e. not returning from sig handler - - // Add context to context list - int bh_res = bh_list_insert(context_list, context); - assert(bh_res == BH_LIST_SUCCESS); - - if (function->is_import_func) { + if (function->is_import_func) { #if WASM_ENABLE_MULTI_MODULE != 0 - if (function->import_module_inst) { - wasm_interp_call_func_import(module_inst, exec_env, function, - frame); - } - else + if (function->import_module_inst) { + wasm_interp_call_func_import(module_inst, exec_env, function, + frame); + } + else #endif - { - /* it is a native function */ - wasm_interp_call_func_native(module_inst, exec_env, function, - frame); + { + /* it is a native function */ + wasm_interp_call_func_native(module_inst, exec_env, function, + frame); + } } - } - else { + else { #if WASM_ENABLE_LAZY_JIT != 0 - /* Fast JIT to LLVM JIT tier-up is enabled */ + /* Fast JIT to LLVM JIT tier-up is enabled */ #if WASM_ENABLE_FAST_JIT != 0 && WASM_ENABLE_JIT != 0 - /* Fast JIT and LLVM JIT are both enabled, call llvm jit function - if it is compiled, else call fast jit function */ - uint32 func_idx = (uint32)(function - module_inst->e->functions); - if (module_inst->module->func_ptrs_compiled - [func_idx - module_inst->module->import_function_count]) { + /* Fast JIT and LLVM JIT are both enabled, call llvm jit function + if it is compiled, else call fast jit function */ + uint32 func_idx = (uint32)(function - module_inst->e->functions); + if (module_inst->module->func_ptrs_compiled + [func_idx - module_inst->module->import_function_count]) { llvm_jit_call_func_bytecode(module_inst, exec_env, function, argc, argv); - /* For llvm jit, the results have been stored in argv, - no need to copy them from stack frame again */ - copy_argv_from_frame = false; - } - else { + /* For llvm jit, the results have been stored in argv, + no need to copy them from stack frame again */ + copy_argv_from_frame = false; + } + else { fast_jit_call_func_bytecode(module_inst, exec_env, function, frame); - } + } #elif WASM_ENABLE_JIT != 0 - /* Only LLVM JIT is enabled */ - llvm_jit_call_func_bytecode(module_inst, exec_env, function, argc, - argv); - /* For llvm jit, the results have been stored in argv, - no need to copy them from stack frame again */ - copy_argv_from_frame = false; + /* Only LLVM JIT is enabled */ + llvm_jit_call_func_bytecode(module_inst, exec_env, function, argc, + argv); + /* For llvm jit, the results have been stored in argv, + no need to copy them from stack frame again */ + copy_argv_from_frame = false; #elif WASM_ENABLE_FAST_JIT != 0 - /* Only Fast JIT is enabled */ - fast_jit_call_func_bytecode(module_inst, exec_env, function, frame); + /* Only Fast JIT is enabled */ + fast_jit_call_func_bytecode(module_inst, exec_env, function, frame); #else - /* Both Fast JIT and LLVM JIT are disabled */ + /* Both Fast JIT and LLVM JIT are disabled */ wasm_interp_call_func_bytecode(module_inst, exec_env, function, frame); #endif #else /* else of WASM_ENABLE_LAZY_JIT != 0 */ - /* Fast JIT to LLVM JIT tier-up is enabled */ + /* Fast JIT to LLVM JIT tier-up is enabled */ #if WASM_ENABLE_JIT != 0 - /* LLVM JIT is enabled */ - llvm_jit_call_func_bytecode(module_inst, exec_env, function, argc, - argv); - /* For llvm jit, the results have been stored in argv, - no need to copy them from stack frame again */ - copy_argv_from_frame = false; + /* LLVM JIT is enabled */ + llvm_jit_call_func_bytecode(module_inst, exec_env, function, argc, + argv); + /* For llvm jit, the results have been stored in argv, + no need to copy them from stack frame again */ + copy_argv_from_frame = false; #elif WASM_ENABLE_FAST_JIT != 0 - /* Fast JIT is enabled */ - fast_jit_call_func_bytecode(module_inst, exec_env, function, frame); + /* Fast JIT is enabled */ + fast_jit_call_func_bytecode(module_inst, exec_env, function, frame); #else - /* Both Fast JIT and LLVM JIT are disabled */ + /* Both Fast JIT and LLVM JIT are disabled */ wasm_interp_call_func_bytecode(module_inst, exec_env, function, frame); #endif #endif /* end of WASM_ENABLE_LAZY_JIT != 0 */ - (void)wasm_interp_call_func_bytecode; + (void)wasm_interp_call_func_bytecode; #if WASM_ENABLE_FAST_JIT != 0 - (void)fast_jit_call_func_bytecode; + (void)fast_jit_call_func_bytecode; #endif } } + korp_tid self = pthread_self(); printf("exit context=%p %p\n", context, (void *)self); - int bh_res = bh_list_remove(context_list, context); - assert(bh_res == BH_LIST_SUCCESS); + os_rm_context(exec_env->handle, context); + /* Output the return value to the caller */ if (!wasm_get_exception(module_inst)) { if (copy_argv_from_frame) { diff --git a/core/shared/platform/common/posix/posix_thread.c b/core/shared/platform/common/posix/posix_thread.c index 3e683f3eb8..399436696c 100644 --- a/core/shared/platform/common/posix/posix_thread.c +++ b/core/shared/platform/common/posix/posix_thread.c @@ -14,6 +14,18 @@ void *contexts = NULL; +static uint32 +thread_handle_hash(void *handle) +{ + return (uint32)(uintptr_t)handle; +} + +static bool +thread_handle_equal(void *h1, void *h2) +{ + return (uint32)(uintptr_t)h1 == (uint32)(uintptr_t)h2 ? true : false; +} + typedef struct { thread_start_routine_t start; void *arg; @@ -66,6 +78,67 @@ handler(int sig) siglongjmp(*context, 1); } +sigjmp_buf * +os_save_context(korp_tid handle) +{ + // Create map + // NOTE: The list is needed for nested calls to `wasm_interp_call_wasm` + if (contexts == NULL) { + contexts = bh_hash_map_create(32, false, (HashFunc)thread_handle_hash, + (KeyEqualFunc)thread_handle_equal, NULL, + wasm_runtime_free); + } + + // Get or create list of contexts for current thread + bh_list *context_list = + bh_hash_map_find(contexts, (void *)(uintptr_t)handle); + if (!context_list) { + context_list = malloc(sizeof(bh_list)); + int ret = bh_list_init(context_list); + assert(ret == BH_LIST_SUCCESS); + + bool bh_ret = bh_hash_map_insert(contexts, (void *)(uintptr_t)handle, + context_list); + assert(bh_ret); + } + assert(context_list); + + // Create context + sigjmp_buf *context = malloc(sizeof(sigjmp_buf)); + pthread_t self = pthread_self(); + printf("context=%p %ld\n", context, self); + return context; +} + +void +os_rm_context(korp_tid handle, sigjmp_buf *context) +{ + korp_tid self = pthread_self(); + bh_list *context_list = bh_hash_map_find(contexts, (void *)(uintptr_t)self); + assert(context_list); + int bh_res = bh_list_remove(context_list, context); + assert(bh_res == BH_LIST_SUCCESS); +} + +bool +os_is_exception(korp_tid handle, sigjmp_buf *context) +{ + if (sigsetjmp(*context, 1) == 0) { + // Normal execution, i.e. not returning from sig handler + + bh_list *context_list = + bh_hash_map_find(contexts, (void *)(uintptr_t)handle); + assert(context_list); + + // Add context to context list + int bh_res = bh_list_insert(context_list, context); + assert(bh_res == BH_LIST_SUCCESS); + return false; + } + + return true; +} + int os_thread_create_with_prio(korp_tid *tid, thread_start_routine_t start, void *arg, unsigned int stack_size, int prio) diff --git a/core/shared/platform/include/platform_api_extension.h b/core/shared/platform/include/platform_api_extension.h index 0a40801d55..96bee97f5e 100644 --- a/core/shared/platform/include/platform_api_extension.h +++ b/core/shared/platform/include/platform_api_extension.h @@ -55,6 +55,16 @@ os_thread_create(korp_tid *p_tid, thread_start_routine_t start, void *arg, int os_thread_signal(korp_tid p_tid, int sig); +sigjmp_buf * +os_save_context(korp_tid handle); + +void +os_rm_context(korp_tid handle, sigjmp_buf *context); + +bool +os_is_exception(korp_tid handle, sigjmp_buf *context); + +void /** * Creates a thread with priority * From 70030cfcf8d843d9f5e191f04feafd241e2d5655 Mon Sep 17 00:00:00 2001 From: eloparco Date: Sat, 28 Jan 2023 22:54:44 +0000 Subject: [PATCH 3/7] feat(wasi-threads): add mutex to protect context data structure --- core/iwasm/common/wasm_application.c | 1 + .../libraries/thread-mgr/thread_manager.c | 2 +- .../platform/common/posix/posix_thread.c | 50 +++++++++++++++---- .../platform/include/platform_api_extension.h | 6 +++ 4 files changed, 49 insertions(+), 10 deletions(-) diff --git a/core/iwasm/common/wasm_application.c b/core/iwasm/common/wasm_application.c index 8445652f64..e76fde4553 100644 --- a/core/iwasm/common/wasm_application.c +++ b/core/iwasm/common/wasm_application.c @@ -210,6 +210,7 @@ wasm_application_execute_main(WASMModuleInstanceCommon *module_inst, int32 argc, WASMExecEnv *exec_env; #endif + os_contexts_init(); ret = execute_main(module_inst, argc, argv); #if WASM_ENABLE_THREAD_MGR != 0 diff --git a/core/iwasm/libraries/thread-mgr/thread_manager.c b/core/iwasm/libraries/thread-mgr/thread_manager.c index 013aa075c8..ee219d9e5a 100644 --- a/core/iwasm/libraries/thread-mgr/thread_manager.c +++ b/core/iwasm/libraries/thread-mgr/thread_manager.c @@ -1061,7 +1061,7 @@ set_exception_visitor(void *node, void *user_data) wasm_inst->cur_exception, sizeof(wasm_inst->cur_exception)); if (curr_exec_env->handle) - os_thread_signal(curr_exec_env->handle, SIGTERM); + os_thread_signal(curr_exec_env->handle, SIGUSR1); /* Terminate the thread so it can exit from dead loops */ set_thread_cancel_flags(curr_exec_env); diff --git a/core/shared/platform/common/posix/posix_thread.c b/core/shared/platform/common/posix/posix_thread.c index 399436696c..89179c5477 100644 --- a/core/shared/platform/common/posix/posix_thread.c +++ b/core/shared/platform/common/posix/posix_thread.c @@ -13,6 +13,7 @@ #include void *contexts = NULL; +korp_mutex context_lock; static uint32 thread_handle_hash(void *handle) @@ -67,20 +68,24 @@ os_thread_wrapper(void *arg) static void handler(int sig) { + assert(sig == SIGUSR1); korp_tid self = pthread_self(); + + os_mutex_lock(&context_lock); bh_list *context_list = bh_hash_map_find(contexts, (void *)(uintptr_t)self); assert(context_list); // Get latest context (first in the list) for current thread sigjmp_buf *context = bh_list_first_elem(context_list); assert(context); + os_mutex_unlock(&context_lock); - printf("handler, %p %ld\n", context, self); siglongjmp(*context, 1); } sigjmp_buf * os_save_context(korp_tid handle) { + os_mutex_lock(&context_lock); // Create map // NOTE: The list is needed for nested calls to `wasm_interp_call_wasm` if (contexts == NULL) { @@ -106,18 +111,51 @@ os_save_context(korp_tid handle) // Create context sigjmp_buf *context = malloc(sizeof(sigjmp_buf)); pthread_t self = pthread_self(); + os_mutex_unlock(&context_lock); + printf("context=%p %ld\n", context, self); return context; } +bool +os_contexts_init() +{ + struct sigaction act; + memset(&act, 0, sizeof(act)); + act.sa_handler = handler; + sigfillset(&act.sa_mask); + if (sigaction(SIGUSR1, &act, NULL) < 0) { + LOG_ERROR("failed to set signal handler"); + return BHT_ERROR; + } + + if (os_mutex_init(&context_lock) != 0) { + LOG_ERROR("failed to init context mutex"); + return false; + } + + return true; +} + +void +os_clean_contexts() +{ + os_mutex_destroy(&context_lock); + + // TODO(eloparco): Release memory +} + void os_rm_context(korp_tid handle, sigjmp_buf *context) { korp_tid self = pthread_self(); + + os_mutex_lock(&context_lock); bh_list *context_list = bh_hash_map_find(contexts, (void *)(uintptr_t)self); assert(context_list); int bh_res = bh_list_remove(context_list, context); assert(bh_res == BH_LIST_SUCCESS); + os_mutex_unlock(&context_lock); } bool @@ -126,6 +164,7 @@ os_is_exception(korp_tid handle, sigjmp_buf *context) if (sigsetjmp(*context, 1) == 0) { // Normal execution, i.e. not returning from sig handler + os_mutex_lock(&context_lock); bh_list *context_list = bh_hash_map_find(contexts, (void *)(uintptr_t)handle); assert(context_list); @@ -133,6 +172,7 @@ os_is_exception(korp_tid handle, sigjmp_buf *context) // Add context to context list int bh_res = bh_list_insert(context_list, context); assert(bh_res == BH_LIST_SUCCESS); + os_mutex_unlock(&context_lock); return false; } @@ -146,14 +186,6 @@ os_thread_create_with_prio(korp_tid *tid, thread_start_routine_t start, pthread_attr_t tattr; thread_wrapper_arg *targ; - struct sigaction act; - memset(&act, 0, sizeof(act)); - act.sa_handler = handler; - act.sa_flags = SA_RESETHAND; - sigfillset(&act.sa_mask); - if (sigaction(SIGTERM, &act, NULL) < 0) - return BHT_ERROR; - assert(stack_size > 0); assert(tid); assert(start); diff --git a/core/shared/platform/include/platform_api_extension.h b/core/shared/platform/include/platform_api_extension.h index 96bee97f5e..c2a734162b 100644 --- a/core/shared/platform/include/platform_api_extension.h +++ b/core/shared/platform/include/platform_api_extension.h @@ -19,6 +19,7 @@ extern "C" { #include extern void *contexts; +extern korp_mutex context_lock; /*************************************************** * * @@ -55,6 +56,9 @@ os_thread_create(korp_tid *p_tid, thread_start_routine_t start, void *arg, int os_thread_signal(korp_tid p_tid, int sig); +bool +os_contexts_init(); + sigjmp_buf * os_save_context(korp_tid handle); @@ -65,6 +69,8 @@ bool os_is_exception(korp_tid handle, sigjmp_buf *context); void +os_clean_contexts(); + /** * Creates a thread with priority * From ef6183ba7409be269436d7378e5ce7198cbcb7c0 Mon Sep 17 00:00:00 2001 From: eloparco Date: Sun, 29 Jan 2023 13:22:00 +0000 Subject: [PATCH 4/7] refactor(wasi-threads): refactor platform functions for stack contexts --- core/iwasm/common/wasm_application.c | 1 - core/iwasm/interpreter/wasm_interp_classic.c | 10 +- .../lib-pthread/lib_pthread_wrapper.c | 12 -- .../platform/common/posix/posix_thread.c | 144 +++++++++--------- .../platform/include/platform_api_extension.h | 79 +++++++--- core/shared/utils/bh_common.h | 12 ++ product-mini/platforms/posix/main.c | 3 + 7 files changed, 148 insertions(+), 113 deletions(-) diff --git a/core/iwasm/common/wasm_application.c b/core/iwasm/common/wasm_application.c index e76fde4553..8445652f64 100644 --- a/core/iwasm/common/wasm_application.c +++ b/core/iwasm/common/wasm_application.c @@ -210,7 +210,6 @@ wasm_application_execute_main(WASMModuleInstanceCommon *module_inst, int32 argc, WASMExecEnv *exec_env; #endif - os_contexts_init(); ret = execute_main(module_inst, argc, argv); #if WASM_ENABLE_THREAD_MGR != 0 diff --git a/core/iwasm/interpreter/wasm_interp_classic.c b/core/iwasm/interpreter/wasm_interp_classic.c index 2722e6dc73..fd4b26252b 100644 --- a/core/iwasm/interpreter/wasm_interp_classic.c +++ b/core/iwasm/interpreter/wasm_interp_classic.c @@ -4183,9 +4183,8 @@ wasm_interp_call_wasm(WASMModuleInstance *module_inst, WASMExecEnv *exec_env, wasm_exec_env_set_cur_frame(exec_env, frame); - sigjmp_buf *context = os_save_context(exec_env->handle); - if (!os_is_exception(exec_env->handle, context)) { - + sigjmp_buf *context = os_create_stack_context(); + if (!os_is_returning_from_signal(exec_env->handle, context)) { if (function->is_import_func) { #if WASM_ENABLE_MULTI_MODULE != 0 if (function->import_module_inst) { @@ -4263,10 +4262,7 @@ wasm_interp_call_wasm(WASMModuleInstance *module_inst, WASMExecEnv *exec_env, #endif } } - - korp_tid self = pthread_self(); - printf("exit context=%p %p\n", context, (void *)self); - os_rm_context(exec_env->handle, context); + os_remove_stack_context(exec_env->handle, context); /* Output the return value to the caller */ if (!wasm_get_exception(module_inst)) { diff --git a/core/iwasm/libraries/lib-pthread/lib_pthread_wrapper.c b/core/iwasm/libraries/lib-pthread/lib_pthread_wrapper.c index d4d9023e34..44dfbded35 100644 --- a/core/iwasm/libraries/lib-pthread/lib_pthread_wrapper.c +++ b/core/iwasm/libraries/lib-pthread/lib_pthread_wrapper.c @@ -150,18 +150,6 @@ static uint32 handle_id = 1; static void lib_pthread_destroy_callback(WASMCluster *cluster); -static uint32 -thread_handle_hash(void *handle) -{ - return (uint32)(uintptr_t)handle; -} - -static bool -thread_handle_equal(void *h1, void *h2) -{ - return (uint32)(uintptr_t)h1 == (uint32)(uintptr_t)h2 ? true : false; -} - static void thread_info_destroy(void *node) { diff --git a/core/shared/platform/common/posix/posix_thread.c b/core/shared/platform/common/posix/posix_thread.c index 89179c5477..a430d1705b 100644 --- a/core/shared/platform/common/posix/posix_thread.c +++ b/core/shared/platform/common/posix/posix_thread.c @@ -12,20 +12,7 @@ #include "bh_hashmap.h" #include -void *contexts = NULL; -korp_mutex context_lock; - -static uint32 -thread_handle_hash(void *handle) -{ - return (uint32)(uintptr_t)handle; -} - -static bool -thread_handle_equal(void *h1, void *h2) -{ - return (uint32)(uintptr_t)h1 == (uint32)(uintptr_t)h2 ? true : false; -} +bh_stack_context_handler_t stack_context_handler; typedef struct { thread_start_routine_t start; @@ -66,117 +53,122 @@ os_thread_wrapper(void *arg) } static void -handler(int sig) +stack_context_sig_handler(int sig) { assert(sig == SIGUSR1); korp_tid self = pthread_self(); - os_mutex_lock(&context_lock); - bh_list *context_list = bh_hash_map_find(contexts, (void *)(uintptr_t)self); + os_mutex_lock(&stack_context_handler.lock); + + /* Get latest stack context (first in the list) for current thread */ + bh_list *context_list = bh_hash_map_find(stack_context_handler.contexts, + (void *)(uintptr_t)self); assert(context_list); - // Get latest context (first in the list) for current thread sigjmp_buf *context = bh_list_first_elem(context_list); assert(context); - os_mutex_unlock(&context_lock); + os_mutex_unlock(&stack_context_handler.lock); siglongjmp(*context, 1); } sigjmp_buf * -os_save_context(korp_tid handle) -{ - os_mutex_lock(&context_lock); - // Create map - // NOTE: The list is needed for nested calls to `wasm_interp_call_wasm` - if (contexts == NULL) { - contexts = bh_hash_map_create(32, false, (HashFunc)thread_handle_hash, - (KeyEqualFunc)thread_handle_equal, NULL, - wasm_runtime_free); - } - - // Get or create list of contexts for current thread - bh_list *context_list = - bh_hash_map_find(contexts, (void *)(uintptr_t)handle); - if (!context_list) { - context_list = malloc(sizeof(bh_list)); - int ret = bh_list_init(context_list); - assert(ret == BH_LIST_SUCCESS); - - bool bh_ret = bh_hash_map_insert(contexts, (void *)(uintptr_t)handle, - context_list); - assert(bh_ret); - } - assert(context_list); - - // Create context - sigjmp_buf *context = malloc(sizeof(sigjmp_buf)); - pthread_t self = pthread_self(); - os_mutex_unlock(&context_lock); - - printf("context=%p %ld\n", context, self); - return context; +os_create_stack_context(korp_tid handle) +{ + return BH_MALLOC(sizeof(sigjmp_buf)); } bool -os_contexts_init() +os_stack_contexts_init() { struct sigaction act; memset(&act, 0, sizeof(act)); - act.sa_handler = handler; + act.sa_handler = stack_context_sig_handler; sigfillset(&act.sa_mask); if (sigaction(SIGUSR1, &act, NULL) < 0) { LOG_ERROR("failed to set signal handler"); - return BHT_ERROR; + return false; } - if (os_mutex_init(&context_lock) != 0) { + if (os_mutex_init(&stack_context_handler.lock) != 0) { LOG_ERROR("failed to init context mutex"); return false; } + stack_context_handler.contexts = bh_hash_map_create( + 32, false, (HashFunc)thread_handle_hash, + (KeyEqualFunc)thread_handle_equal, NULL, wasm_runtime_free); + if (stack_context_handler.contexts == NULL) { + LOG_ERROR("failed to init stack contexts"); + return false; + } + return true; } void -os_clean_contexts() +os_stack_contexts_deinit() { - os_mutex_destroy(&context_lock); + os_mutex_destroy(&stack_context_handler.lock); // TODO(eloparco): Release memory } void -os_rm_context(korp_tid handle, sigjmp_buf *context) +os_remove_stack_context(korp_tid handle, sigjmp_buf *context) { korp_tid self = pthread_self(); - os_mutex_lock(&context_lock); - bh_list *context_list = bh_hash_map_find(contexts, (void *)(uintptr_t)self); + os_mutex_lock(&stack_context_handler.lock); + + bh_list *context_list = bh_hash_map_find(stack_context_handler.contexts, + (void *)(uintptr_t)self); assert(context_list); - int bh_res = bh_list_remove(context_list, context); - assert(bh_res == BH_LIST_SUCCESS); - os_mutex_unlock(&context_lock); + int ret = bh_list_remove(context_list, context); + assert(ret == BH_LIST_SUCCESS); + + os_mutex_unlock(&stack_context_handler.lock); } bool -os_is_exception(korp_tid handle, sigjmp_buf *context) +os_is_returning_from_signal(korp_tid handle, sigjmp_buf *context) { - if (sigsetjmp(*context, 1) == 0) { - // Normal execution, i.e. not returning from sig handler + if (sigsetjmp(*context, 1)) + return true; - os_mutex_lock(&context_lock); - bh_list *context_list = - bh_hash_map_find(contexts, (void *)(uintptr_t)handle); - assert(context_list); + os_mutex_lock(&stack_context_handler.lock); - // Add context to context list - int bh_res = bh_list_insert(context_list, context); - assert(bh_res == BH_LIST_SUCCESS); - os_mutex_unlock(&context_lock); - return false; + /* Get or create list of stack contexts for current thread */ + bh_list *context_list = bh_hash_map_find(stack_context_handler.contexts, + (void *)(uintptr_t)handle); + if (!context_list) { + context_list = BH_MALLOC(sizeof(bh_list)); + if (!context_list) { + os_mutex_unlock(&stack_context_handler.lock); + return NULL; + } + int ret_list = bh_list_init(context_list); + if (ret_list != BH_LIST_SUCCESS) { + BH_FREE(context_list); + os_mutex_unlock(&stack_context_handler.lock); + return NULL; + } + + bool ret_map = + bh_hash_map_insert(stack_context_handler.contexts, + (void *)(uintptr_t)handle, context_list); + if (!ret_map) { + BH_FREE(context_list); + os_mutex_unlock(&stack_context_handler.lock); + return NULL; + } } - return true; + /* Add stack context to stack context list */ + int bh_res = bh_list_insert(context_list, context); + assert(bh_res == BH_LIST_SUCCESS); + + os_mutex_unlock(&stack_context_handler.lock); + return false; } int diff --git a/core/shared/platform/include/platform_api_extension.h b/core/shared/platform/include/platform_api_extension.h index c2a734162b..4b5b627084 100644 --- a/core/shared/platform/include/platform_api_extension.h +++ b/core/shared/platform/include/platform_api_extension.h @@ -18,8 +18,19 @@ extern "C" { #endif #include -extern void *contexts; -extern korp_mutex context_lock; +/** + * @brief Structure used to track stack contexts. It contains a map to save the stack contexts for each thread. + * A list is used to handles stack contexts created in nested functions (for the + * same thread). Stack contexts are used by the signal handler to restore a + * previously saved state. + * + */ +typedef struct { + void *contexts; + korp_mutex lock; +} bh_stack_context_handler_t; +extern bh_stack_context_handler_t stack_context_handler; /*************************************************** * * @@ -56,21 +67,6 @@ os_thread_create(korp_tid *p_tid, thread_start_routine_t start, void *arg, int os_thread_signal(korp_tid p_tid, int sig); -bool -os_contexts_init(); - -sigjmp_buf * -os_save_context(korp_tid handle); - -void -os_rm_context(korp_tid handle, sigjmp_buf *context); - -bool -os_is_exception(korp_tid handle, sigjmp_buf *context); - -void -os_clean_contexts(); - /** * Creates a thread with priority * @@ -302,6 +298,55 @@ os_sem_getvalue(korp_sem *sem, int *sval); int os_sem_unlink(const char *name); +/****************************************** + * Functions to handle stack contexts + ******************************************/ + +/** + * @brief Initializes the data structures to handle stack contexts + * + * @return true if success, false otherwise + */ +bool +os_stack_contexts_init(); + +/** + * @brief Destroys the data structures used for stack contexts + * + */ +void +os_stack_contexts_deinit(); + +/** + * @brief Creates and returns an empty stack context for the current thread + * + * @return sigjmp_buf* pointer to stack context created + */ +sigjmp_buf * +os_create_stack_context(); + +/** + * @brief Removes the stack context received from the stack contexts tracked for + * the specified thread. Called before the stack context is invalidated + * by the function (in which the stack context was created) returning. + * + * @param handle thread to remove the context from + * @param stack_context pointer to the stack pointer to remove + */ +void +os_remove_stack_context(korp_tid handle, sigjmp_buf *stack_context); + +/** + * @brief Returns true if returning from a signal handler; otherwise, adds the + * stack context received to the tracked stack contexts for the specified thread + * + * @param handle thread in which the context was created + * @param stack_context pointer to the stack pointer to use + * @return true if returning from a signal handler, false otherwise + */ +bool +os_is_returning_from_signal(korp_tid handle, sigjmp_buf *stack_context); + /**************************************************** * Section 2 * * Socket support * diff --git a/core/shared/utils/bh_common.h b/core/shared/utils/bh_common.h index edb962eb1c..52d65ca231 100644 --- a/core/shared/utils/bh_common.h +++ b/core/shared/utils/bh_common.h @@ -66,6 +66,18 @@ bh_strdup(const char *s); char * wa_strdup(const char *s); +static inline uint32 +thread_handle_hash(void *handle) +{ + return (uint32)(uintptr_t)handle; +} + +static inline bool +thread_handle_equal(void *h1, void *h2) +{ + return (uint32)(uintptr_t)h1 == (uint32)(uintptr_t)h2 ? true : false; +} + #ifdef __cplusplus } #endif diff --git a/product-mini/platforms/posix/main.c b/product-mini/platforms/posix/main.c index e04745c68d..809002091a 100644 --- a/product-mini/platforms/posix/main.c +++ b/product-mini/platforms/posix/main.c @@ -621,6 +621,9 @@ main(int argc, char *argv[]) ns_lookup_pool_size); #endif + if (!os_stack_contexts_init()) + goto fail3; + /* instantiate the module */ if (!(wasm_module_inst = wasm_runtime_instantiate(wasm_module, stack_size, heap_size, From 669d7f1b88b9497a260dc168f1584d31bcddaf2a Mon Sep 17 00:00:00 2001 From: eloparco Date: Sun, 29 Jan 2023 15:35:03 +0000 Subject: [PATCH 5/7] fix(wasi-threads): release stack context memory before exit --- .../platform/common/posix/posix_thread.c | 53 ++++++++++++++----- .../platform/include/platform_api_extension.h | 2 +- product-mini/platforms/posix/main.c | 2 + 3 files changed, 43 insertions(+), 14 deletions(-) diff --git a/core/shared/platform/common/posix/posix_thread.c b/core/shared/platform/common/posix/posix_thread.c index a430d1705b..c13f31707b 100644 --- a/core/shared/platform/common/posix/posix_thread.c +++ b/core/shared/platform/common/posix/posix_thread.c @@ -77,6 +77,21 @@ os_create_stack_context(korp_tid handle) return BH_MALLOC(sizeof(sigjmp_buf)); } +static void +free_stack_context_list(void *stack_context_list) +{ + bh_list *context_list = (bh_list *)stack_context_list; + + sigjmp_buf *context = bh_list_first_elem(context_list); + while (context) { + sigjmp_buf *next = bh_list_elem_next(context); + BH_FREE(context); + context = next; + }; + + BH_FREE(context_list); +} + bool os_stack_contexts_init() { @@ -96,7 +111,7 @@ os_stack_contexts_init() stack_context_handler.contexts = bh_hash_map_create( 32, false, (HashFunc)thread_handle_hash, - (KeyEqualFunc)thread_handle_equal, NULL, wasm_runtime_free); + (KeyEqualFunc)thread_handle_equal, NULL, free_stack_context_list); if (stack_context_handler.contexts == NULL) { LOG_ERROR("failed to init stack contexts"); return false; @@ -106,11 +121,16 @@ os_stack_contexts_init() } void -os_stack_contexts_deinit() +os_stack_contexts_destroy() { - os_mutex_destroy(&stack_context_handler.lock); + os_mutex_lock(&stack_context_handler.lock); + if (!bh_hash_map_destroy(stack_context_handler.contexts)) + LOG_ERROR("failed to destroy stack context map"); + stack_context_handler.contexts = NULL; + os_mutex_unlock(&stack_context_handler.lock); - // TODO(eloparco): Release memory + if (os_mutex_destroy(&stack_context_handler.lock) != BHT_OK) + LOG_ERROR("failed to destroy stack contexts"); } void @@ -125,6 +145,7 @@ os_remove_stack_context(korp_tid handle, sigjmp_buf *context) assert(context_list); int ret = bh_list_remove(context_list, context); assert(ret == BH_LIST_SUCCESS); + BH_FREE(context); os_mutex_unlock(&stack_context_handler.lock); } @@ -143,30 +164,36 @@ os_is_returning_from_signal(korp_tid handle, sigjmp_buf *context) if (!context_list) { context_list = BH_MALLOC(sizeof(bh_list)); if (!context_list) { - os_mutex_unlock(&stack_context_handler.lock); - return NULL; + LOG_ERROR("failed to allocate stack context list"); + goto unlock; } + int ret_list = bh_list_init(context_list); if (ret_list != BH_LIST_SUCCESS) { - BH_FREE(context_list); - os_mutex_unlock(&stack_context_handler.lock); - return NULL; + LOG_ERROR("failed to initialize stack context list"); + goto free_list_and_unlock; } bool ret_map = bh_hash_map_insert(stack_context_handler.contexts, (void *)(uintptr_t)handle, context_list); if (!ret_map) { - BH_FREE(context_list); - os_mutex_unlock(&stack_context_handler.lock); - return NULL; + LOG_ERROR("failed to insert stack context list into map"); + goto free_list_and_unlock; } } /* Add stack context to stack context list */ int bh_res = bh_list_insert(context_list, context); - assert(bh_res == BH_LIST_SUCCESS); + if (bh_res != BH_LIST_SUCCESS) { + LOG_ERROR("failed to insert stack context into list"); + goto free_list_and_unlock; + } + goto unlock; +free_list_and_unlock: + BH_FREE(context_list); +unlock: os_mutex_unlock(&stack_context_handler.lock); return false; } diff --git a/core/shared/platform/include/platform_api_extension.h b/core/shared/platform/include/platform_api_extension.h index 4b5b627084..80f18b6f5b 100644 --- a/core/shared/platform/include/platform_api_extension.h +++ b/core/shared/platform/include/platform_api_extension.h @@ -315,7 +315,7 @@ os_stack_contexts_init(); * */ void -os_stack_contexts_deinit(); +os_stack_contexts_destroy(); /** * @brief Creates and returns an empty stack context for the current thread diff --git a/product-mini/platforms/posix/main.c b/product-mini/platforms/posix/main.c index 809002091a..2911f32ce7 100644 --- a/product-mini/platforms/posix/main.c +++ b/product-mini/platforms/posix/main.c @@ -695,6 +695,8 @@ main(int argc, char *argv[]) /* unload the native libraries */ unregister_and_unload_native_libs(native_handle_count, native_handle_list); #endif + + os_stack_contexts_destroy(); /* destroy runtime environment */ wasm_runtime_destroy(); From d70da2f019c178d036d027eb59be0e0a2874c709 Mon Sep 17 00:00:00 2001 From: eloparco Date: Sun, 29 Jan 2023 23:15:41 +0000 Subject: [PATCH 6/7] fix(wasi-threads): restore no_pthread sample and update thread_termination sample to use blocking operations --- .../platform/include/platform_api_extension.h | 1 - product-mini/platforms/posix/main.c | 2 +- samples/wasi-threads/wasm-apps/no_pthread.c | 28 +++++++++++-------- .../wasm-apps/thread_termination.c | 9 ++++-- 4 files changed, 23 insertions(+), 17 deletions(-) diff --git a/core/shared/platform/include/platform_api_extension.h b/core/shared/platform/include/platform_api_extension.h index 80f18b6f5b..ce75698aea 100644 --- a/core/shared/platform/include/platform_api_extension.h +++ b/core/shared/platform/include/platform_api_extension.h @@ -24,7 +24,6 @@ extern "C" { * A list is used to handles stack contexts created in nested functions (for the * same thread). Stack contexts are used by the signal handler to restore a * previously saved state. - * */ typedef struct { void *contexts; diff --git a/product-mini/platforms/posix/main.c b/product-mini/platforms/posix/main.c index 2911f32ce7..64ec64b373 100644 --- a/product-mini/platforms/posix/main.c +++ b/product-mini/platforms/posix/main.c @@ -695,7 +695,7 @@ main(int argc, char *argv[]) /* unload the native libraries */ unregister_and_unload_native_libs(native_handle_count, native_handle_list); #endif - + os_stack_contexts_destroy(); /* destroy runtime environment */ diff --git a/samples/wasi-threads/wasm-apps/no_pthread.c b/samples/wasi-threads/wasm-apps/no_pthread.c index 277013a0a3..fcf64a7b76 100644 --- a/samples/wasi-threads/wasm-apps/no_pthread.c +++ b/samples/wasi-threads/wasm-apps/no_pthread.c @@ -12,8 +12,6 @@ #include "wasi_thread_start.h" -#define STOP_FROM_MAIN 1 - static const int64_t SECOND = 1000 * 1000 * 1000; typedef struct { @@ -31,11 +29,12 @@ __wasi_thread_start_C(int thread_id, int *start_arg) printf("New thread ID: %d, starting parameter: %d\n", thread_id, data->value); -#if STOP_FROM_MAIN == 1 - __builtin_wasm_memory_atomic_wait32(0, 0, -1); -#else - __wasi_proc_exit(55); -#endif + data->thread_id = thread_id; + data->value += 8; + printf("Updated value: %d\n", data->value); + + data->th_ready = 1; + __builtin_wasm_memory_atomic_notify(&data->th_ready, 1); } int @@ -57,11 +56,16 @@ main(int argc, char **argv) goto final; } -#if STOP_FROM_MAIN == 1 - __wasi_proc_exit(56); -#else - __builtin_wasm_memory_atomic_wait32(0, 0, -1); -#endif + if (__builtin_wasm_memory_atomic_wait32(&data.th_ready, 0, SECOND) == 2) { + printf("Timeout\n"); + ret = EXIT_FAILURE; + goto final; + } + + printf("Thread completed, new value: %d, thread id: %d\n", data.value, + data.thread_id); + + assert(thread_id == data.thread_id); final: start_args_deinit(&data.base); diff --git a/samples/wasi-threads/wasm-apps/thread_termination.c b/samples/wasi-threads/wasm-apps/thread_termination.c index dfc228eb3c..77d7e38a60 100644 --- a/samples/wasi-threads/wasm-apps/thread_termination.c +++ b/samples/wasi-threads/wasm-apps/thread_termination.c @@ -17,6 +17,7 @@ #define TEST_TERMINATION_BY_TRAP 0 // Otherwise test `proc_exit` termination #define TEST_TERMINATION_IN_MAIN_THREAD 1 +#define TEST_ATOMIC_WAIT 1 // Otherwise test `sleep` #define TIMEOUT_SECONDS 10 #define NUM_THREADS 3 @@ -30,9 +31,11 @@ typedef struct { void run_long_task() { - // Busy waiting to be interruptible by trap or `proc_exit` - for (int i = 0; i < TIMEOUT_SECONDS; i++) - sleep(1); +#if TEST_ATOMIC_WAIT == 1 + __builtin_wasm_memory_atomic_wait32(0, 0, -1); +#else + sleep(TIMEOUT_SECONDS); +#endif } void From 57d208d2386667413232398f58f2ee854afb169b Mon Sep 17 00:00:00 2001 From: eloparco Date: Tue, 31 Jan 2023 02:18:23 +0000 Subject: [PATCH 7/7] fix(wasi-threads): do not use mutex in signal handler --- .../platform/common/posix/posix_thread.c | 36 +++++++++++++------ 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/core/shared/platform/common/posix/posix_thread.c b/core/shared/platform/common/posix/posix_thread.c index c13f31707b..8aad1d4f37 100644 --- a/core/shared/platform/common/posix/posix_thread.c +++ b/core/shared/platform/common/posix/posix_thread.c @@ -12,6 +12,8 @@ #include "bh_hashmap.h" #include +#define STACK_CTX_SIG SIGUSR1 +static os_thread_local_attribute sigset_t newmask, oldmask; bh_stack_context_handler_t stack_context_handler; typedef struct { @@ -55,11 +57,9 @@ os_thread_wrapper(void *arg) static void stack_context_sig_handler(int sig) { - assert(sig == SIGUSR1); + assert(sig == STACK_CTX_SIG); korp_tid self = pthread_self(); - os_mutex_lock(&stack_context_handler.lock); - /* Get latest stack context (first in the list) for current thread */ bh_list *context_list = bh_hash_map_find(stack_context_handler.contexts, (void *)(uintptr_t)self); @@ -67,7 +67,6 @@ stack_context_sig_handler(int sig) sigjmp_buf *context = bh_list_first_elem(context_list); assert(context); - os_mutex_unlock(&stack_context_handler.lock); siglongjmp(*context, 1); } @@ -92,6 +91,23 @@ free_stack_context_list(void *stack_context_list) BH_FREE(context_list); } +static inline void +stack_ctx_block_sig_and_lock() +{ + sigemptyset(&newmask); + sigaddset(&newmask, STACK_CTX_SIG); + pthread_sigmask(SIG_BLOCK, &newmask, &oldmask); + + os_mutex_lock(&stack_context_handler.lock); +} + +static inline void +stack_ctx_unlock_and_unblock_sig() +{ + os_mutex_unlock(&stack_context_handler.lock); + pthread_sigmask(SIG_SETMASK, &oldmask, NULL); +} + bool os_stack_contexts_init() { @@ -99,7 +115,7 @@ os_stack_contexts_init() memset(&act, 0, sizeof(act)); act.sa_handler = stack_context_sig_handler; sigfillset(&act.sa_mask); - if (sigaction(SIGUSR1, &act, NULL) < 0) { + if (sigaction(STACK_CTX_SIG, &act, NULL) < 0) { LOG_ERROR("failed to set signal handler"); return false; } @@ -123,11 +139,9 @@ os_stack_contexts_init() void os_stack_contexts_destroy() { - os_mutex_lock(&stack_context_handler.lock); if (!bh_hash_map_destroy(stack_context_handler.contexts)) LOG_ERROR("failed to destroy stack context map"); stack_context_handler.contexts = NULL; - os_mutex_unlock(&stack_context_handler.lock); if (os_mutex_destroy(&stack_context_handler.lock) != BHT_OK) LOG_ERROR("failed to destroy stack contexts"); @@ -138,7 +152,7 @@ os_remove_stack_context(korp_tid handle, sigjmp_buf *context) { korp_tid self = pthread_self(); - os_mutex_lock(&stack_context_handler.lock); + stack_ctx_block_sig_and_lock(); bh_list *context_list = bh_hash_map_find(stack_context_handler.contexts, (void *)(uintptr_t)self); @@ -147,7 +161,7 @@ os_remove_stack_context(korp_tid handle, sigjmp_buf *context) assert(ret == BH_LIST_SUCCESS); BH_FREE(context); - os_mutex_unlock(&stack_context_handler.lock); + stack_ctx_unlock_and_unblock_sig(); } bool @@ -156,7 +170,7 @@ os_is_returning_from_signal(korp_tid handle, sigjmp_buf *context) if (sigsetjmp(*context, 1)) return true; - os_mutex_lock(&stack_context_handler.lock); + stack_ctx_block_sig_and_lock(); /* Get or create list of stack contexts for current thread */ bh_list *context_list = bh_hash_map_find(stack_context_handler.contexts, @@ -194,7 +208,7 @@ os_is_returning_from_signal(korp_tid handle, sigjmp_buf *context) free_list_and_unlock: BH_FREE(context_list); unlock: - os_mutex_unlock(&stack_context_handler.lock); + stack_ctx_unlock_and_unblock_sig(); return false; }