Skip to content

Commit e516de8

Browse files
authored
Fix possible data race in thread manager (bytecodealliance#1973)
Destroy child thread's exec_env before destroying its module instance and add the process into cluster's lock to avoid possible data race: if exec_env is removed from custer's exec_env_list and destroyed later, the main thread may not wait it and start to destroy the wasm runtime, and the destroying of the sub thread's exec_env may free or overread/written an destroyed or re-initialized resource. And fix an issue in wasm_cluster_cancel_thread.
1 parent 739acfc commit e516de8

File tree

1 file changed

+27
-13
lines changed

1 file changed

+27
-13
lines changed

core/iwasm/libraries/thread-mgr/thread_manager.c

Lines changed: 27 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -545,14 +545,18 @@ wasm_cluster_destroy_spawned_exec_env(WASMExecEnv *exec_env)
545545
wasm_module_inst_t module_inst = wasm_runtime_get_module_inst(exec_env);
546546
bh_assert(cluster != NULL);
547547

548-
/* Free aux stack space */
549548
os_mutex_lock(&cluster->lock);
549+
550+
/* Free aux stack space */
550551
free_aux_stack(exec_env, exec_env->aux_stack_bottom.bottom);
552+
/* Remove exec_env */
551553
wasm_cluster_del_exec_env(cluster, exec_env);
552-
os_mutex_unlock(&cluster->lock);
554+
/* Destroy exec_env */
553555
wasm_exec_env_destroy_internal(exec_env);
554-
556+
/* Routine exit, destroy instance */
555557
wasm_runtime_deinstantiate_internal(module_inst, true);
558+
559+
os_mutex_unlock(&cluster->lock);
556560
}
557561

558562
/* start routine of thread manager */
@@ -585,16 +589,17 @@ thread_manager_start_routine(void *arg)
585589
#endif
586590

587591
os_mutex_lock(&cluster->lock);
592+
588593
/* Free aux stack space */
589594
free_aux_stack(exec_env, exec_env->aux_stack_bottom.bottom);
590-
/* routine exit, destroy instance */
591-
wasm_runtime_deinstantiate_internal(module_inst, true);
592-
/* Remove and exec_env */
595+
/* Remove exec_env */
593596
wasm_cluster_del_exec_env(cluster, exec_env);
594-
os_mutex_unlock(&cluster->lock);
595-
596-
/* destroy exec_env */
597+
/* Destroy exec_env */
597598
wasm_exec_env_destroy_internal(exec_env);
599+
/* Routine exit, destroy instance */
600+
wasm_runtime_deinstantiate_internal(module_inst, true);
601+
602+
os_mutex_unlock(&cluster->lock);
598603

599604
os_thread_exit(ret);
600605
return ret;
@@ -909,13 +914,19 @@ wasm_cluster_exit_thread(WASMExecEnv *exec_env, void *retval)
909914
/* App exit the thread, free the resources before exit native thread */
910915
/* Detach the native thread here to ensure the resources are freed */
911916
wasm_cluster_detach_thread(exec_env);
917+
912918
os_mutex_lock(&cluster->lock);
919+
913920
/* Free aux stack space */
914921
free_aux_stack(exec_env, exec_env->aux_stack_bottom.bottom);
915-
/* Remove and destroy exec_env */
922+
/* Remove exec_env */
916923
wasm_cluster_del_exec_env(cluster, exec_env);
917-
os_mutex_unlock(&cluster->lock);
924+
/* Destroy exec_env */
918925
wasm_exec_env_destroy_internal(exec_env);
926+
/* Routine exit, destroy instance */
927+
wasm_runtime_deinstantiate_internal(exec_env->module_inst, true);
928+
929+
os_mutex_unlock(&cluster->lock);
919930

920931
os_thread_exit(retval);
921932
}
@@ -935,11 +946,14 @@ int32
935946
wasm_cluster_cancel_thread(WASMExecEnv *exec_env)
936947
{
937948
os_mutex_lock(&cluster_list_lock);
938-
os_mutex_lock(&exec_env->cluster->lock);
939949

940950
if (!exec_env->cluster) {
941-
goto final;
951+
os_mutex_unlock(&cluster_list_lock);
952+
return 0;
942953
}
954+
955+
os_mutex_lock(&exec_env->cluster->lock);
956+
943957
if (!clusters_have_exec_env(exec_env)) {
944958
/* Invalid thread or the thread has exited */
945959
goto final;

0 commit comments

Comments
 (0)