From 5a23ae465c58e3613e9dda1f676df52d93aa1356 Mon Sep 17 00:00:00 2001 From: Christof Petig <33882057+cpetig@users.noreply.github.com> Date: Fri, 5 May 2023 04:01:58 +0200 Subject: [PATCH] Fix three multi-threading and wasm-c-api-imports issues (#2173) Fix issue reported in #2172: wasm-c-api `wasm_func_call` may use a wrong exec_env when multi-threading is enabled, with error "invalid exec env" reported Fix issue reported in #2149: main instance's `c_api_func_imports` are not passed to the counterpart of new thread's instance in wasi-threads mode Fix issue of invalid size calculated to copy `c_api_func_imports` in pthread mode And refactor the code to use `wasm_cluster_dup_c_api_imports` to copy the `c_api_func_imports` to new thread for wasi-threads mode and pthread mode. --- core/iwasm/common/wasm_c_api.c | 15 +++++- .../lib-pthread/lib_pthread_wrapper.c | 41 +--------------- .../lib_wasi_threads_wrapper.c | 3 ++ .../libraries/thread-mgr/thread_manager.c | 49 +++++++++++++++++++ .../libraries/thread-mgr/thread_manager.h | 5 ++ 5 files changed, 73 insertions(+), 40 deletions(-) diff --git a/core/iwasm/common/wasm_c_api.c b/core/iwasm/common/wasm_c_api.c index 639980ca2..7b8cf4779 100644 --- a/core/iwasm/common/wasm_c_api.c +++ b/core/iwasm/common/wasm_c_api.c @@ -23,6 +23,9 @@ #if WASM_ENABLE_WASM_CACHE != 0 #include #endif +#if WASM_ENABLE_THREAD_MGR != 0 +#include "thread_manager.h" +#endif /* * Thread Model: @@ -3315,7 +3318,17 @@ wasm_func_call(const wasm_func_t *func, const wasm_val_vec_t *params, goto failed; } - exec_env = wasm_runtime_get_exec_env_singleton(func->inst_comm_rt); +#ifdef OS_ENABLE_HW_BOUND_CHECK + exec_env = wasm_runtime_get_exec_env_tls(); +#endif +#if WASM_ENABLE_THREAD_MGR != 0 + if (!exec_env) { + exec_env = wasm_clusters_search_exec_env(func->inst_comm_rt); + } +#endif + if (!exec_env) { + exec_env = wasm_runtime_get_exec_env_singleton(func->inst_comm_rt); + } if (!exec_env) { goto failed; } diff --git a/core/iwasm/libraries/lib-pthread/lib_pthread_wrapper.c b/core/iwasm/libraries/lib-pthread/lib_pthread_wrapper.c index fc82e969e..ae1fd94f7 100644 --- a/core/iwasm/libraries/lib-pthread/lib_pthread_wrapper.c +++ b/core/iwasm/libraries/lib-pthread/lib_pthread_wrapper.c @@ -594,45 +594,8 @@ pthread_create_wrapper(wasm_exec_env_t exec_env, wasm_runtime_set_wasi_ctx(new_module_inst, wasi_ctx); #endif - /* workaround about passing instantiate-linking information */ - { - CApiFuncImport *c_api_func_imports; - uint32 import_func_count = 0; - uint32 size_in_bytes = 0; - -#if WASM_ENABLE_INTERP != 0 - if (module_inst->module_type == Wasm_Module_Bytecode) { - new_c_api_func_imports = &( - ((WASMModuleInstance *)new_module_inst)->e->c_api_func_imports); - c_api_func_imports = - ((WASMModuleInstance *)module_inst)->e->c_api_func_imports; - import_func_count = ((WASMModule *)module)->import_function_count; - } -#endif -#if WASM_ENABLE_AOT != 0 - if (module_inst->module_type == Wasm_Module_AoT) { - AOTModuleInstanceExtra *e = - (AOTModuleInstanceExtra *)((AOTModuleInstance *)new_module_inst) - ->e; - new_c_api_func_imports = &(e->c_api_func_imports); - - e = (AOTModuleInstanceExtra *)((AOTModuleInstance *)module_inst)->e; - c_api_func_imports = e->c_api_func_imports; - - import_func_count = ((AOTModule *)module)->import_func_count; - } -#endif - - if (import_func_count != 0 && c_api_func_imports) { - size_in_bytes = sizeof(CApiFuncImport *) * import_func_count; - *new_c_api_func_imports = wasm_runtime_malloc(size_in_bytes); - if (!(*new_c_api_func_imports)) - goto fail; - - bh_memcpy_s(*new_c_api_func_imports, size_in_bytes, - c_api_func_imports, size_in_bytes); - } - } + if (!(wasm_cluster_dup_c_api_imports(new_module_inst, module_inst))) + goto fail; if (!(info_node = wasm_runtime_malloc(sizeof(ThreadInfoNode)))) goto fail; diff --git a/core/iwasm/libraries/lib-wasi-threads/lib_wasi_threads_wrapper.c b/core/iwasm/libraries/lib-wasi-threads/lib_wasi_threads_wrapper.c index 30d66076d..6b36c9073 100644 --- a/core/iwasm/libraries/lib-wasi-threads/lib_wasi_threads_wrapper.c +++ b/core/iwasm/libraries/lib-wasi-threads/lib_wasi_threads_wrapper.c @@ -96,6 +96,9 @@ thread_spawn_wrapper(wasm_exec_env_t exec_env, uint32 start_arg) wasm_runtime_set_custom_data_internal( new_module_inst, wasm_runtime_get_custom_data(module_inst)); + if (!(wasm_cluster_dup_c_api_imports(new_module_inst, module_inst))) + goto thread_preparation_fail; + #if WASM_ENABLE_LIBC_WASI != 0 wasi_ctx = wasm_runtime_get_wasi_ctx(module_inst); if (wasi_ctx) diff --git a/core/iwasm/libraries/thread-mgr/thread_manager.c b/core/iwasm/libraries/thread-mgr/thread_manager.c index bfb89c089..9303eb3f5 100644 --- a/core/iwasm/libraries/thread-mgr/thread_manager.c +++ b/core/iwasm/libraries/thread-mgr/thread_manager.c @@ -733,6 +733,55 @@ fail1: return -1; } +bool +wasm_cluster_dup_c_api_imports(WASMModuleInstanceCommon *module_inst_dst, + const WASMModuleInstanceCommon *module_inst_src) +{ + /* workaround about passing instantiate-linking information */ + CApiFuncImport **new_c_api_func_imports = NULL; + CApiFuncImport *c_api_func_imports; + uint32 import_func_count = 0; + uint32 size_in_bytes = 0; + +#if WASM_ENABLE_INTERP != 0 + if (module_inst_src->module_type == Wasm_Module_Bytecode) { + new_c_api_func_imports = + &(((WASMModuleInstance *)module_inst_dst)->e->c_api_func_imports); + c_api_func_imports = ((const WASMModuleInstance *)module_inst_src) + ->e->c_api_func_imports; + import_func_count = + ((WASMModule *)(((const WASMModuleInstance *)module_inst_src) + ->module)) + ->import_function_count; + } +#endif +#if WASM_ENABLE_AOT != 0 + if (module_inst_src->module_type == Wasm_Module_AoT) { + AOTModuleInstanceExtra *e = + (AOTModuleInstanceExtra *)((AOTModuleInstance *)module_inst_dst)->e; + new_c_api_func_imports = &(e->c_api_func_imports); + + e = (AOTModuleInstanceExtra *)((AOTModuleInstance *)module_inst_src)->e; + c_api_func_imports = e->c_api_func_imports; + + import_func_count = + ((AOTModule *)(((AOTModuleInstance *)module_inst_src)->module)) + ->import_func_count; + } +#endif + + if (import_func_count != 0 && c_api_func_imports) { + size_in_bytes = sizeof(CApiFuncImport) * import_func_count; + *new_c_api_func_imports = wasm_runtime_malloc(size_in_bytes); + if (!(*new_c_api_func_imports)) + return false; + + bh_memcpy_s(*new_c_api_func_imports, size_in_bytes, c_api_func_imports, + size_in_bytes); + } + return true; +} + #if WASM_ENABLE_DEBUG_INTERP != 0 WASMCurrentEnvStatus * wasm_cluster_create_exenv_status() diff --git a/core/iwasm/libraries/thread-mgr/thread_manager.h b/core/iwasm/libraries/thread-mgr/thread_manager.h index 899a4cc21..2060869c2 100644 --- a/core/iwasm/libraries/thread-mgr/thread_manager.h +++ b/core/iwasm/libraries/thread-mgr/thread_manager.h @@ -74,6 +74,11 @@ wasm_cluster_destroy(WASMCluster *cluster); WASMCluster * wasm_exec_env_get_cluster(WASMExecEnv *exec_env); +/* Forward registered functions to a new thread */ +bool +wasm_cluster_dup_c_api_imports(WASMModuleInstanceCommon *module_inst_dst, + const WASMModuleInstanceCommon *module_inst_src); + int32 wasm_cluster_create_thread(WASMExecEnv *exec_env, wasm_module_inst_t module_inst, bool alloc_aux_stack,