From c7cdb78394ae73026dc0bccc27ac0e9314b5b61e Mon Sep 17 00:00:00 2001 From: Wenyong Huang Date: Fri, 24 Mar 2023 14:05:17 +0800 Subject: [PATCH] Fix issues reported by Coverity (#2053) Fix the potential dead lock issue reported by Coverity code analysis tool. --- core/iwasm/common/wasm_shared_memory.c | 1 + .../libraries/thread-mgr/thread_manager.c | 33 +++++++++++++------ 2 files changed, 24 insertions(+), 10 deletions(-) diff --git a/core/iwasm/common/wasm_shared_memory.c b/core/iwasm/common/wasm_shared_memory.c index 778d6e57..c5e78e43 100644 --- a/core/iwasm/common/wasm_shared_memory.c +++ b/core/iwasm/common/wasm_shared_memory.c @@ -253,6 +253,7 @@ acquire_wait_info(void *address, AtomicWaitNode *wait_node) wait_info->wait_list = &wait_info->wait_list_head; ret = bh_list_init(wait_info->wait_list); bh_assert(ret == BH_LIST_SUCCESS); + (void)ret; if (!bh_hash_map_insert(wait_map, address, (void *)wait_info)) { wasm_runtime_free(wait_info); diff --git a/core/iwasm/libraries/thread-mgr/thread_manager.c b/core/iwasm/libraries/thread-mgr/thread_manager.c index 107186ed..bfb89c08 100644 --- a/core/iwasm/libraries/thread-mgr/thread_manager.c +++ b/core/iwasm/libraries/thread-mgr/thread_manager.c @@ -382,9 +382,9 @@ wasm_cluster_add_exec_env(WASMCluster *cluster, WASMExecEnv *exec_env) return ret; } -/* The caller should lock cluster->lock for thread safety */ -bool -wasm_cluster_del_exec_env(WASMCluster *cluster, WASMExecEnv *exec_env) +static bool +wasm_cluster_del_exec_env_internal(WASMCluster *cluster, WASMExecEnv *exec_env, + bool can_destroy_cluster) { bool ret = true; bh_assert(exec_env->cluster == cluster); @@ -407,13 +407,26 @@ wasm_cluster_del_exec_env(WASMCluster *cluster, WASMExecEnv *exec_env) if (bh_list_remove(&cluster->exec_env_list, exec_env) != 0) ret = false; - if (cluster->exec_env_list.len == 0) { - /* exec_env_list empty, destroy the cluster */ - wasm_cluster_destroy(cluster); + if (can_destroy_cluster) { + if (cluster->exec_env_list.len == 0) { + /* exec_env_list empty, destroy the cluster */ + wasm_cluster_destroy(cluster); + } } + else { + /* Don't destroy cluster as cluster->lock is being used */ + } + return ret; } +/* The caller should lock cluster->lock for thread safety */ +bool +wasm_cluster_del_exec_env(WASMCluster *cluster, WASMExecEnv *exec_env) +{ + return wasm_cluster_del_exec_env_internal(cluster, exec_env, true); +} + static WASMExecEnv * wasm_cluster_search_exec_env(WASMCluster *cluster, WASMModuleInstanceCommon *module_inst) @@ -561,7 +574,7 @@ wasm_cluster_destroy_spawned_exec_env(WASMExecEnv *exec_env) /* Free aux stack space */ free_aux_stack(exec_env, exec_env->aux_stack_bottom.bottom); /* Remove exec_env */ - wasm_cluster_del_exec_env(cluster, exec_env); + wasm_cluster_del_exec_env_internal(cluster, exec_env, false); /* Destroy exec_env */ wasm_exec_env_destroy_internal(exec_env); /* Routine exit, destroy instance */ @@ -621,7 +634,7 @@ thread_manager_start_routine(void *arg) /* Free aux stack space */ free_aux_stack(exec_env, exec_env->aux_stack_bottom.bottom); /* Remove exec_env */ - wasm_cluster_del_exec_env(cluster, exec_env); + wasm_cluster_del_exec_env_internal(cluster, exec_env, false); /* Destroy exec_env */ wasm_exec_env_destroy_internal(exec_env); /* Routine exit, destroy instance */ @@ -707,7 +720,7 @@ wasm_cluster_create_thread(WASMExecEnv *exec_env, return 0; fail4: - wasm_cluster_del_exec_env(cluster, new_exec_env); + wasm_cluster_del_exec_env_internal(cluster, new_exec_env, false); fail3: /* free the allocated aux stack space */ if (alloc_aux_stack) @@ -972,7 +985,7 @@ wasm_cluster_exit_thread(WASMExecEnv *exec_env, void *retval) /* Free aux stack space */ free_aux_stack(exec_env, exec_env->aux_stack_bottom.bottom); /* Remove exec_env */ - wasm_cluster_del_exec_env(cluster, exec_env); + wasm_cluster_del_exec_env_internal(cluster, exec_env, false); /* Destroy exec_env */ wasm_exec_env_destroy_internal(exec_env); /* Routine exit, destroy instance */