From 092efbfe219063c7854bf6298216eaf828871d73 Mon Sep 17 00:00:00 2001 From: Wenyong Huang Date: Mon, 17 Jan 2022 17:09:04 +0800 Subject: [PATCH] Fix thread manager issues (#962) Fix the issue that joining a detached thread might result in joining hang, resolve the issue by adding wait_count for a thread's exec_env to indicate whether a thread needs to detach itself or not when it exits. And add checks for the input exec_env for cluster's join/detach/cancel thread. --- core/iwasm/common/wasm_exec_env.h | 2 + .../libraries/thread-mgr/thread_manager.c | 65 ++++++++++++++++++- .../libraries/thread-mgr/thread_manager.h | 1 + 3 files changed, 66 insertions(+), 2 deletions(-) diff --git a/core/iwasm/common/wasm_exec_env.h b/core/iwasm/common/wasm_exec_env.h index ea2fafd6..86ad1c3e 100644 --- a/core/iwasm/common/wasm_exec_env.h +++ b/core/iwasm/common/wasm_exec_env.h @@ -98,6 +98,8 @@ typedef struct WASMExecEnv { /* used to support debugger */ korp_mutex wait_lock; korp_cond wait_cond; + /* the count of threads which are joining current thread */ + uint32 wait_count; #endif #if WASM_ENABLE_DEBUG_INTERP != 0 diff --git a/core/iwasm/libraries/thread-mgr/thread_manager.c b/core/iwasm/libraries/thread-mgr/thread_manager.c index f6987c0c..8db457c2 100644 --- a/core/iwasm/libraries/thread-mgr/thread_manager.c +++ b/core/iwasm/libraries/thread-mgr/thread_manager.c @@ -625,16 +625,69 @@ wasm_cluster_set_debug_inst(WASMCluster *cluster, WASMDebugInstance *inst) #endif /* end of WASM_ENABLE_DEBUG_INTERP */ +/* Check whether the exec_env is in one of all clusters, the caller + should add lock to the cluster list before calling us */ +static bool +clusters_have_exec_env(WASMExecEnv *exec_env) +{ + WASMCluster *cluster = bh_list_first_elem(cluster_list); + WASMExecEnv *node; + + while (cluster) { + node = bh_list_first_elem(&cluster->exec_env_list); + + while (node) { + if (node == exec_env) { + bh_assert(exec_env->cluster == cluster); + return true; + } + node = bh_list_elem_next(node); + } + + cluster = bh_list_elem_next(cluster); + } + + return false; +} + int32 wasm_cluster_join_thread(WASMExecEnv *exec_env, void **ret_val) { - return os_thread_join(exec_env->handle, ret_val); + korp_tid handle; + + os_mutex_lock(&cluster_list_lock); + if (!clusters_have_exec_env(exec_env)) { + /* Invalid thread or the thread has exited */ + if (ret_val) + *ret_val = NULL; + os_mutex_unlock(&cluster_list_lock); + return 0; + } + exec_env->wait_count++; + handle = exec_env->handle; + os_mutex_unlock(&cluster_list_lock); + return os_thread_join(handle, ret_val); } int32 wasm_cluster_detach_thread(WASMExecEnv *exec_env) { - return os_thread_detach(exec_env->handle); + int32 ret = 0; + + os_mutex_lock(&cluster_list_lock); + if (!clusters_have_exec_env(exec_env)) { + /* Invalid thread or the thread has exited */ + os_mutex_unlock(&cluster_list_lock); + return 0; + } + if (exec_env->wait_count == 0) { + /* Only detach current thread when there is no other thread + joining it, otherwise let the system resources for the + thread be released after joining */ + ret = os_thread_detach(exec_env->handle); + } + os_mutex_unlock(&cluster_list_lock); + return ret; } void @@ -680,6 +733,14 @@ wasm_cluster_exit_thread(WASMExecEnv *exec_env, void *retval) int32 wasm_cluster_cancel_thread(WASMExecEnv *exec_env) { + os_mutex_lock(&cluster_list_lock); + if (!clusters_have_exec_env(exec_env)) { + /* Invalid thread or the thread has exited */ + os_mutex_unlock(&cluster_list_lock); + return 0; + } + os_mutex_unlock(&cluster_list_lock); + /* Set the termination flag */ #if WASM_ENABLE_DEBUG_INTERP != 0 wasm_cluster_thread_send_signal(exec_env, WAMR_SIG_TERM); diff --git a/core/iwasm/libraries/thread-mgr/thread_manager.h b/core/iwasm/libraries/thread-mgr/thread_manager.h index e99efc99..2d4962a2 100644 --- a/core/iwasm/libraries/thread-mgr/thread_manager.h +++ b/core/iwasm/libraries/thread-mgr/thread_manager.h @@ -19,6 +19,7 @@ extern "C" { #if WASM_ENABLE_DEBUG_INTERP != 0 typedef struct WASMDebugInstance WASMDebugInstance; #endif + typedef struct WASMCluster { struct WASMCluster *next;