mirror of
				https://github.com/bytecodealliance/wasm-micro-runtime.git
				synced 2025-10-31 13:17:31 +00:00 
			
		
		
		
	Stop abusing shared memory lock to protect exception (#2509)
Use a separate global lock instead. Fixes: https://github.com/bytecodealliance/wasm-micro-runtime/issues/2407
This commit is contained in:
		
							parent
							
								
									53d7027de0
								
							
						
					
					
						commit
						382d52fc05
					
				|  | @ -2377,12 +2377,7 @@ wasm_runtime_get_exec_env_singleton(WASMModuleInstanceCommon *module_inst_comm) | |||
| void | ||||
| wasm_set_exception(WASMModuleInstance *module_inst, const char *exception) | ||||
| { | ||||
|     WASMExecEnv *exec_env = NULL; | ||||
| 
 | ||||
| #if WASM_ENABLE_SHARED_MEMORY != 0 | ||||
|     if (module_inst->memory_count > 0) | ||||
|         shared_memory_lock(module_inst->memories[0]); | ||||
| #endif | ||||
|     exception_lock(module_inst); | ||||
|     if (exception) { | ||||
|         snprintf(module_inst->cur_exception, sizeof(module_inst->cur_exception), | ||||
|                  "Exception: %s", exception); | ||||
|  | @ -2390,19 +2385,14 @@ wasm_set_exception(WASMModuleInstance *module_inst, const char *exception) | |||
|     else { | ||||
|         module_inst->cur_exception[0] = '\0'; | ||||
|     } | ||||
| #if WASM_ENABLE_SHARED_MEMORY != 0 | ||||
|     if (module_inst->memory_count > 0) | ||||
|         shared_memory_unlock(module_inst->memories[0]); | ||||
| #endif | ||||
|     exception_unlock(module_inst); | ||||
| 
 | ||||
| #if WASM_ENABLE_THREAD_MGR != 0 | ||||
|     exec_env = | ||||
|     WASMExecEnv *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, exception); | ||||
|     } | ||||
| #else | ||||
|     (void)exec_env; | ||||
| #endif | ||||
| } | ||||
| 
 | ||||
|  | @ -2453,10 +2443,7 @@ wasm_copy_exception(WASMModuleInstance *module_inst, char *exception_buf) | |||
| { | ||||
|     bool has_exception = false; | ||||
| 
 | ||||
| #if WASM_ENABLE_SHARED_MEMORY != 0 | ||||
|     if (module_inst->memory_count > 0) | ||||
|         shared_memory_lock(module_inst->memories[0]); | ||||
| #endif | ||||
|     exception_lock(module_inst); | ||||
|     if (module_inst->cur_exception[0] != '\0') { | ||||
|         /* NULL is passed if the caller is not interested in getting the
 | ||||
|          * exception content, but only in knowing if an exception has been | ||||
|  | @ -2468,10 +2455,7 @@ wasm_copy_exception(WASMModuleInstance *module_inst, char *exception_buf) | |||
|                         sizeof(module_inst->cur_exception)); | ||||
|         has_exception = true; | ||||
|     } | ||||
| #if WASM_ENABLE_SHARED_MEMORY != 0 | ||||
|     if (module_inst->memory_count > 0) | ||||
|         shared_memory_unlock(module_inst->memories[0]); | ||||
| #endif | ||||
|     exception_unlock(module_inst); | ||||
| 
 | ||||
|     return has_exception; | ||||
| } | ||||
|  |  | |||
|  | @ -668,6 +668,16 @@ void | |||
| wasm_propagate_wasi_args(WASMModule *module); | ||||
| #endif | ||||
| 
 | ||||
| #if WASM_ENABLE_THREAD_MGR != 0 | ||||
| void | ||||
| exception_lock(WASMModuleInstance *module_inst); | ||||
| void | ||||
| exception_unlock(WASMModuleInstance *module_inst); | ||||
| #else | ||||
| #define exception_lock(module_inst) (void)(module_inst) | ||||
| #define exception_unlock(module_inst) (void)(module_inst) | ||||
| #endif | ||||
| 
 | ||||
| #ifdef __cplusplus | ||||
| } | ||||
| #endif | ||||
|  |  | |||
|  | @ -16,10 +16,6 @@ | |||
| #include "debug_engine.h" | ||||
| #endif | ||||
| 
 | ||||
| #if WASM_ENABLE_SHARED_MEMORY != 0 | ||||
| #include "wasm_shared_memory.h" | ||||
| #endif | ||||
| 
 | ||||
| typedef struct { | ||||
|     bh_list_link l; | ||||
|     void (*destroy_cb)(WASMCluster *); | ||||
|  | @ -32,6 +28,8 @@ static bh_list cluster_list_head; | |||
| static bh_list *const cluster_list = &cluster_list_head; | ||||
| static korp_mutex cluster_list_lock; | ||||
| 
 | ||||
| static korp_mutex _exception_lock; | ||||
| 
 | ||||
| typedef void (*list_visitor)(void *, void *); | ||||
| 
 | ||||
| static uint32 cluster_max_thread_num = CLUSTER_MAX_THREAD_NUM; | ||||
|  | @ -52,6 +50,10 @@ thread_manager_init() | |||
|         return false; | ||||
|     if (os_mutex_init(&cluster_list_lock) != 0) | ||||
|         return false; | ||||
|     if (os_mutex_init(&_exception_lock) != 0) { | ||||
|         os_mutex_destroy(&cluster_list_lock); | ||||
|         return false; | ||||
|     } | ||||
|     return true; | ||||
| } | ||||
| 
 | ||||
|  | @ -66,6 +68,7 @@ thread_manager_destroy() | |||
|         cluster = next; | ||||
|     } | ||||
|     wasm_cluster_cancel_all_callbacks(); | ||||
|     os_mutex_destroy(&_exception_lock); | ||||
|     os_mutex_destroy(&cluster_list_lock); | ||||
| } | ||||
| 
 | ||||
|  | @ -1240,72 +1243,56 @@ wasm_cluster_resume_all(WASMCluster *cluster) | |||
|     os_mutex_unlock(&cluster->lock); | ||||
| } | ||||
| 
 | ||||
| struct spread_exception_data { | ||||
|     WASMExecEnv *skip; | ||||
|     const char *exception; | ||||
| }; | ||||
| 
 | ||||
| static void | ||||
| set_exception_visitor(void *node, void *user_data) | ||||
| { | ||||
|     WASMExecEnv *curr_exec_env = (WASMExecEnv *)node; | ||||
|     WASMExecEnv *exec_env = (WASMExecEnv *)user_data; | ||||
|     WASMModuleInstanceCommon *module_inst = get_module_inst(exec_env); | ||||
|     WASMModuleInstance *wasm_inst = (WASMModuleInstance *)module_inst; | ||||
|     const struct spread_exception_data *data = user_data; | ||||
|     WASMExecEnv *exec_env = (WASMExecEnv *)node; | ||||
| 
 | ||||
|     if (curr_exec_env != exec_env) { | ||||
|         WASMModuleInstance *curr_wasm_inst = | ||||
|             (WASMModuleInstance *)get_module_inst(curr_exec_env); | ||||
|     if (exec_env != data->skip) { | ||||
|         WASMModuleInstance *wasm_inst = | ||||
|             (WASMModuleInstance *)get_module_inst(exec_env); | ||||
| 
 | ||||
|         /* Only spread non "wasi proc exit" exception */ | ||||
| #if WASM_ENABLE_SHARED_MEMORY != 0 | ||||
|         if (curr_wasm_inst->memory_count > 0) | ||||
|             shared_memory_lock(curr_wasm_inst->memories[0]); | ||||
| #endif | ||||
|         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)); | ||||
|         exception_lock(wasm_inst); | ||||
|         if (data->exception != NULL) { | ||||
|             /* Only spread non "wasi proc exit" exception */ | ||||
|             if (strcmp(data->exception, "wasi proc exit")) { | ||||
|                 snprintf(wasm_inst->cur_exception, | ||||
|                          sizeof(wasm_inst->cur_exception), "Exception: %s", | ||||
|                          data->exception); | ||||
|             } | ||||
|         } | ||||
| #if WASM_ENABLE_SHARED_MEMORY != 0 | ||||
|         if (curr_wasm_inst->memory_count > 0) | ||||
|             shared_memory_unlock(curr_wasm_inst->memories[0]); | ||||
| #endif | ||||
|         else { | ||||
|             wasm_inst->cur_exception[0] = '\0'; | ||||
|         } | ||||
|         exception_unlock(wasm_inst); | ||||
| 
 | ||||
|         /* 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); | ||||
| 
 | ||||
| #if WASM_ENABLE_SHARED_MEMORY != 0 | ||||
|         if (curr_wasm_inst->memory_count > 0) | ||||
|             shared_memory_lock(curr_wasm_inst->memories[0]); | ||||
| #endif | ||||
|         curr_wasm_inst->cur_exception[0] = '\0'; | ||||
| #if WASM_ENABLE_SHARED_MEMORY != 0 | ||||
|         if (curr_wasm_inst->memory_count > 0) | ||||
|             shared_memory_unlock(curr_wasm_inst->memories[0]); | ||||
| #endif | ||||
|         if (data->exception != NULL) { | ||||
|             set_thread_cancel_flags(exec_env); | ||||
|         } | ||||
|     } | ||||
| } | ||||
| 
 | ||||
| void | ||||
| wasm_cluster_spread_exception(WASMExecEnv *exec_env, bool clear) | ||||
| wasm_cluster_spread_exception(WASMExecEnv *exec_env, const char *exception) | ||||
| { | ||||
|     const bool has_exception = exception != NULL; | ||||
|     WASMCluster *cluster = wasm_exec_env_get_cluster(exec_env); | ||||
|     bh_assert(cluster); | ||||
| 
 | ||||
|     struct spread_exception_data data; | ||||
|     data.skip = exec_env; | ||||
|     data.exception = exception; | ||||
| 
 | ||||
|     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 = has_exception; | ||||
|     traverse_list(&cluster->exec_env_list, set_exception_visitor, &data); | ||||
|     os_mutex_unlock(&cluster->lock); | ||||
| } | ||||
| 
 | ||||
|  | @ -1353,3 +1340,21 @@ wasm_cluster_is_thread_terminated(WASMExecEnv *exec_env) | |||
| 
 | ||||
|     return is_thread_terminated; | ||||
| } | ||||
| 
 | ||||
| void | ||||
| exception_lock(WASMModuleInstance *module_inst) | ||||
| { | ||||
|     /*
 | ||||
|      * Note: this lock could be per module instance if desirable. | ||||
|      * We can revisit on AOT version bump. | ||||
|      * It probably doesn't matter though because the exception handling | ||||
|      * logic should not be executed too frequently anyway. | ||||
|      */ | ||||
|     os_mutex_lock(&_exception_lock); | ||||
| } | ||||
| 
 | ||||
| void | ||||
| exception_unlock(WASMModuleInstance *module_inst) | ||||
| { | ||||
|     os_mutex_unlock(&_exception_lock); | ||||
| } | ||||
|  |  | |||
|  | @ -139,7 +139,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, const char *exception); | ||||
| 
 | ||||
| WASMExecEnv * | ||||
| wasm_cluster_spawn_exec_env(WASMExecEnv *exec_env); | ||||
|  |  | |||
		Loading…
	
		Reference in New Issue
	
	Block a user
	 YAMAMOTO Takashi
						YAMAMOTO Takashi