Browse Source

Add lock to protect the operations of accessing exec env (#1991)

Data race may occur when accessing exec_env's fields, e.g. suspend_flags
and handle. Add lock `exec_env->wait_lock` for them to resolve the issue.
Enrico Loparco 2 năm trước cách đây
mục cha
commit
52e26e59cf

+ 7 - 0
core/iwasm/common/wasm_exec_env.c

@@ -208,10 +208,17 @@ void
 wasm_exec_env_set_thread_info(WASMExecEnv *exec_env)
 {
     uint8 *stack_boundary = os_thread_get_stack_boundary();
+
+#if WASM_ENABLE_THREAD_MGR != 0
+    os_mutex_lock(&exec_env->wait_lock);
+#endif
     exec_env->handle = os_self_thread();
     exec_env->native_stack_boundary =
         stack_boundary ? stack_boundary + WASM_STACK_GUARD_SIZE : NULL;
     exec_env->native_stack_top_min = (void *)UINTPTR_MAX;
+#if WASM_ENABLE_THREAD_MGR != 0
+    os_mutex_unlock(&exec_env->wait_lock);
+#endif
 }
 
 #if WASM_ENABLE_THREAD_MGR != 0

+ 6 - 0
core/iwasm/interpreter/wasm_interp_classic.c

@@ -1036,20 +1036,25 @@ wasm_interp_call_func_import(WASMModuleInstance *module_inst,
 #if WASM_ENABLE_DEBUG_INTERP != 0
 #define CHECK_SUSPEND_FLAGS()                                          \
     do {                                                               \
+        os_mutex_lock(&exec_env->wait_lock);                           \
         if (IS_WAMR_TERM_SIG(exec_env->current_status->signal_flag)) { \
+            os_mutex_unlock(&exec_env->wait_lock);                     \
             return;                                                    \
         }                                                              \
         if (IS_WAMR_STOP_SIG(exec_env->current_status->signal_flag)) { \
             SYNC_ALL_TO_FRAME();                                       \
             wasm_cluster_thread_waiting_run(exec_env);                 \
         }                                                              \
+        os_mutex_unlock(&exec_env->wait_lock);                         \
     } while (0)
 #else
 #define CHECK_SUSPEND_FLAGS()                                             \
     do {                                                                  \
+        os_mutex_lock(&exec_env->wait_lock);                              \
         if (exec_env->suspend_flags.flags != 0) {                         \
             if (exec_env->suspend_flags.flags & 0x01) {                   \
                 /* terminate current thread */                            \
+                os_mutex_unlock(&exec_env->wait_lock);                    \
                 return;                                                   \
             }                                                             \
             while (exec_env->suspend_flags.flags & 0x02) {                \
@@ -1057,6 +1062,7 @@ wasm_interp_call_func_import(WASMModuleInstance *module_inst,
                 os_cond_wait(&exec_env->wait_cond, &exec_env->wait_lock); \
             }                                                             \
         }                                                                 \
+        os_mutex_unlock(&exec_env->wait_lock);                            \
     } while (0)
 #endif /* WASM_ENABLE_DEBUG_INTERP */
 #endif /* WASM_ENABLE_THREAD_MGR */

+ 3 - 0
core/iwasm/interpreter/wasm_interp_fast.c

@@ -1054,13 +1054,16 @@ wasm_interp_call_func_import(WASMModuleInstance *module_inst,
 #if WASM_ENABLE_THREAD_MGR != 0
 #define CHECK_SUSPEND_FLAGS()                           \
     do {                                                \
+        os_mutex_lock(&exec_env->wait_lock);            \
         if (exec_env->suspend_flags.flags != 0) {       \
             if (exec_env->suspend_flags.flags & 0x01) { \
                 /* terminate current thread */          \
+                os_mutex_unlock(&exec_env->wait_lock);  \
                 return;                                 \
             }                                           \
             /* TODO: support suspend and breakpoint */  \
         }                                               \
+        os_mutex_unlock(&exec_env->wait_lock);          \
     } while (0)
 #endif
 

+ 0 - 3
core/iwasm/libraries/lib-wasi-threads/lib_wasi_threads_wrapper.c

@@ -123,19 +123,16 @@ thread_spawn_wrapper(wasm_exec_env_t exec_env, uint32 start_arg)
     thread_start_arg->arg = start_arg;
     thread_start_arg->start_func = start_func;
 
-    os_mutex_lock(&exec_env->wait_lock);
     ret = wasm_cluster_create_thread(exec_env, new_module_inst, false,
                                      thread_start, thread_start_arg);
     if (ret != 0) {
         LOG_ERROR("Failed to spawn a new thread");
         goto thread_spawn_fail;
     }
-    os_mutex_unlock(&exec_env->wait_lock);
 
     return thread_id;
 
 thread_spawn_fail:
-    os_mutex_unlock(&exec_env->wait_lock);
     deallocate_thread_id(thread_id);
 
 thread_preparation_fail:

+ 15 - 1
core/iwasm/libraries/thread-mgr/thread_manager.c

@@ -574,12 +574,16 @@ thread_manager_start_routine(void *arg)
     bh_assert(cluster != NULL);
     bh_assert(module_inst != NULL);
 
+    os_mutex_lock(&exec_env->wait_lock);
     exec_env->handle = os_self_thread();
+    os_mutex_unlock(&exec_env->wait_lock);
     ret = exec_env->thread_start_routine(exec_env);
 
 #ifdef OS_ENABLE_HW_BOUND_CHECK
+    os_mutex_lock(&exec_env->wait_lock);
     if (exec_env->suspend_flags.flags & 0x08)
         ret = exec_env->thread_ret_value;
+    os_mutex_unlock(&exec_env->wait_lock);
 #endif
 
     /* Routine exit */
@@ -854,8 +858,11 @@ wasm_cluster_join_thread(WASMExecEnv *exec_env, void **ret_val)
         os_mutex_unlock(&cluster_list_lock);
         return 0;
     }
+
+    os_mutex_lock(&exec_env->wait_lock);
     exec_env->wait_count++;
     handle = exec_env->handle;
+    os_mutex_unlock(&exec_env->wait_lock);
 
     os_mutex_unlock(&exec_env->cluster->lock);
     os_mutex_unlock(&cluster_list_lock);
@@ -936,12 +943,14 @@ wasm_cluster_exit_thread(WASMExecEnv *exec_env, void *retval)
 static void
 set_thread_cancel_flags(WASMExecEnv *exec_env)
 {
+    os_mutex_lock(&exec_env->wait_lock);
     /* Set the termination flag */
 #if WASM_ENABLE_DEBUG_INTERP != 0
     wasm_cluster_thread_send_signal(exec_env, WAMR_SIG_TERM);
 #else
     exec_env->suspend_flags.flags |= 0x01;
 #endif
+    os_mutex_unlock(&exec_env->wait_lock);
 }
 
 int32
@@ -1209,5 +1218,10 @@ wasm_cluster_spread_custom_data(WASMModuleInstanceCommon *module_inst,
 bool
 wasm_cluster_is_thread_terminated(WASMExecEnv *exec_env)
 {
-    return (exec_env->suspend_flags.flags & 0x01) ? true : false;
+    os_mutex_lock(&exec_env->wait_lock);
+    bool is_thread_terminated =
+        (exec_env->suspend_flags.flags & 0x01) ? true : false;
+    os_mutex_unlock(&exec_env->wait_lock);
+
+    return is_thread_terminated;
 }