thread suspension test: Fix several issues (#3850)

- Use united macros to access exec_env->suspend_flags
- Acquire global_exec_env_list_lock when accessing exec_env in some places
- Several minor fixes in interpreter

core/iwasm/common/wasm_blocking_op.c

@@ -11,8 +11,10 @@
 
 #if WASM_ENABLE_THREAD_MGR != 0 && defined(OS_ENABLE_WAKEUP_BLOCKING_OP)
 
-#define LOCK(env) WASM_SUSPEND_FLAGS_LOCK((env)->wait_lock)
-#define UNLOCK(env) WASM_SUSPEND_FLAGS_UNLOCK((env)->wait_lock)
+#include "../libraries/thread-mgr/thread_manager.h"
+
+#define LOCK(env) WASM_SUSPEND_FLAGS_LOCK((env)->cluster->thread_state_lock)
+#define UNLOCK(env) WASM_SUSPEND_FLAGS_UNLOCK((env)->cluster->thread_state_lock)
 
 #define ISSET(env, bit)                                                       \
     ((WASM_SUSPEND_FLAGS_GET((env)->suspend_flags) & WASM_SUSPEND_FLAG_##bit) \

core/iwasm/interpreter/wasm_interp_classic.c

@@ -4561,7 +4561,7 @@ wasm_interp_call_func_bytecode(WASMModuleInstance *module,
 
                 if (opcode == WASM_OP_I32_STORE8) {
                     CHECK_MEMORY_OVERFLOW(1);
-                    *(uint8 *)maddr = (uint8)sval;
+                    STORE_U8(maddr, (uint8)sval);
                 }
                 else {
                     CHECK_MEMORY_OVERFLOW(2);
@@ -4587,7 +4587,7 @@ wasm_interp_call_func_bytecode(WASMModuleInstance *module,
 
                 if (opcode == WASM_OP_I64_STORE8) {
                     CHECK_MEMORY_OVERFLOW(1);
-                    *(uint8 *)maddr = (uint8)sval;
+                    STORE_U8(maddr, (uint8)sval);
                 }
                 else if (opcode == WASM_OP_I64_STORE16) {
                     CHECK_MEMORY_OVERFLOW(2);
@@ -6354,9 +6354,11 @@ wasm_interp_call_func_bytecode(WASMModuleInstance *module,
 #if WASM_ENABLE_DEBUG_INTERP != 0
             HANDLE_OP(DEBUG_OP_BREAK)
             {
+                WASM_SUSPEND_FLAGS_LOCK(exec_env->cluster->thread_state_lock);
                 wasm_cluster_thread_send_signal(exec_env, WAMR_SIG_TRAP);
                 WASM_SUSPEND_FLAGS_FETCH_OR(exec_env->suspend_flags,
                                             WASM_SUSPEND_FLAG_SUSPEND);
+                WASM_SUSPEND_FLAGS_UNLOCK(exec_env->cluster->thread_state_lock);
                 frame_ip--;
                 SYNC_ALL_TO_FRAME();
                 CHECK_SUSPEND_FLAGS();

core/iwasm/interpreter/wasm_interp_fast.c

@@ -3763,7 +3763,7 @@ wasm_interp_call_func_bytecode(WASMModuleInstance *module,
                 addr = GET_OPERAND(uint32, I32, 2);
                 frame_ip += 4;
                 CHECK_MEMORY_OVERFLOW(1);
-                STORE_U8(maddr, (uint8_t)sval);
+                STORE_U8(maddr, (uint8)sval);
                 HANDLE_OP_END();
             }
 
@@ -3802,7 +3802,7 @@ wasm_interp_call_func_bytecode(WASMModuleInstance *module,
                 addr = GET_OPERAND(uint32, I32, 2);
                 frame_ip += 4;
                 CHECK_MEMORY_OVERFLOW(1);
-                *(uint8 *)maddr = (uint8)sval;
+                STORE_U8(maddr, (uint8)sval);
                 HANDLE_OP_END();
             }
 

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

@@ -17,6 +17,17 @@
 #include "debug_engine.h"
 #endif
 
+#define SUSPEND_FLAGS_ISSET(exec_env, bit)              \
+    ((WASM_SUSPEND_FLAGS_GET((exec_env)->suspend_flags) \
+      & WASM_SUSPEND_FLAG_##bit)                        \
+     != 0)
+#define SUSPEND_FLAGS_SET(exec_env, bit)                   \
+    WASM_SUSPEND_FLAGS_FETCH_OR((exec_env)->suspend_flags, \
+                                WASM_SUSPEND_FLAG_##bit)
+#define SUSPEND_FLAGS_CLR(exec_env, bit)                    \
+    WASM_SUSPEND_FLAGS_FETCH_AND((exec_env)->suspend_flags, \
+                                 ~WASM_SUSPEND_FLAG_##bit)
+
 typedef struct {
     bh_list_link l;
     void (*destroy_cb)(WASMCluster *);
@@ -181,25 +192,22 @@ get_exec_env_of_current_thread()
     return global_exec_env_list_find_with_tid(handle);
 }
 
+/* Check whether the exec_env is in exec_env_list, or, whehter it is
+   a valid exec_env. The caller must lock global_exec_env_list_lock. */
 static bool
 global_exec_env_list_has_exec_env(WASMExecEnv *exec_env)
 {
     GlobalExecEnvNode *exec_env_node;
 
-    os_mutex_lock(&global_exec_env_list_lock);
-
     exec_env_node = global_exec_env_list;
 
     while (exec_env_node) {
         if (exec_env_node->exec_env == exec_env) {
-            os_mutex_unlock(&global_exec_env_list_lock);
             return true;
         }
         exec_env_node = exec_env_node->next;
     }
 
-    os_mutex_unlock(&global_exec_env_list_lock);
-
     return false;
 }
 
@@ -963,7 +971,7 @@ thread_manager_start_routine(void *arg)
 
 #ifdef OS_ENABLE_HW_BOUND_CHECK
     os_mutex_lock(&cluster->thread_state_lock);
-    if (exec_env->suspend_flags.flags & WASM_SUSPEND_FLAG_EXIT)
+    if (SUSPEND_FLAGS_ISSET(exec_env, EXIT))
         ret = exec_env->thread_ret_value;
     os_mutex_unlock(&cluster->thread_state_lock);
 #endif
@@ -1248,11 +1256,8 @@ 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 */
-static bool
-clusters_have_exec_env(WASMExecEnv *exec_env)
-{
-    return global_exec_env_list_has_exec_env(exec_env);
-}
+#define clusters_have_exec_env(exec_env) \
+    global_exec_env_list_has_exec_env(exec_env)
 
 int32
 wasm_cluster_join_thread(WASMExecEnv *exec_env, WASMExecEnv *self,
@@ -1261,10 +1266,13 @@ wasm_cluster_join_thread(WASMExecEnv *exec_env, WASMExecEnv *self,
     korp_tid handle;
     int32 ret;
 
+    os_mutex_lock(&global_exec_env_list_lock);
+
     if (!clusters_have_exec_env(exec_env)) {
         /* Invalid thread or thread has exited */
         if (ret_val)
             *ret_val = NULL;
+        os_mutex_unlock(&global_exec_env_list_lock);
         return 0;
     }
 
@@ -1275,6 +1283,7 @@ wasm_cluster_join_thread(WASMExecEnv *exec_env, WASMExecEnv *self,
         if (ret_val)
             *ret_val = NULL;
         os_mutex_unlock(&exec_env->wait_lock);
+        os_mutex_unlock(&global_exec_env_list_lock);
         return 0;
     }
 
@@ -1283,6 +1292,8 @@ wasm_cluster_join_thread(WASMExecEnv *exec_env, WASMExecEnv *self,
 
     os_mutex_unlock(&exec_env->wait_lock);
 
+    os_mutex_unlock(&global_exec_env_list_lock);
+
     if (self)
         wasm_thread_change_to_safe(self, WASM_THREAD_VMWAIT);
 
@@ -1299,8 +1310,11 @@ wasm_cluster_detach_thread(WASMExecEnv *exec_env)
 {
     int32 ret = 0;
 
+    os_mutex_lock(&global_exec_env_list_lock);
+
     if (!clusters_have_exec_env(exec_env)) {
         /* Invalid thread or the thread has exited */
+        os_mutex_unlock(&global_exec_env_list_lock);
         return 0;
     }
 
@@ -1318,6 +1332,8 @@ wasm_cluster_detach_thread(WASMExecEnv *exec_env)
     }
     os_mutex_unlock(&exec_env->wait_lock);
 
+    os_mutex_unlock(&global_exec_env_list_lock);
+
     return ret;
 }
 
@@ -1335,7 +1351,7 @@ wasm_cluster_exit_thread(WASMExecEnv *exec_env, void *retval)
         os_mutex_lock(&cluster->thread_state_lock);
         /* Store the return value in exec_env */
         exec_env->thread_ret_value = retval;
-        exec_env->suspend_flags.flags |= WASM_SUSPEND_FLAG_EXIT;
+        SUSPEND_FLAGS_SET(exec_env, EXIT);
         os_mutex_unlock(&cluster->thread_state_lock);
 
 #ifndef BH_PLATFORM_WINDOWS
@@ -1401,7 +1417,7 @@ set_thread_cancel_flags(WASMExecEnv *exec_env)
 #if WASM_ENABLE_DEBUG_INTERP != 0
     wasm_cluster_thread_send_signal(exec_env, WAMR_SIG_TERM);
 #endif
-    exec_env->suspend_flags.flags |= WASM_SUSPEND_FLAG_TERMINATE;
+    SUSPEND_FLAGS_SET(exec_env, TERMINATE);
 
     os_mutex_unlock(&exec_env->cluster->thread_state_lock);
 
@@ -1417,7 +1433,7 @@ clear_thread_cancel_flags(WASMExecEnv *exec_env)
 
     os_mutex_lock(&exec_env->cluster->thread_state_lock);
 
-    exec_env->suspend_flags.flags &= ~WASM_SUSPEND_FLAG_TERMINATE;
+    SUSPEND_FLAGS_CLR(exec_env, TERMINATE);
 
     os_mutex_unlock(&exec_env->cluster->thread_state_lock);
 }
@@ -1425,13 +1441,18 @@ clear_thread_cancel_flags(WASMExecEnv *exec_env)
 int32
 wasm_cluster_cancel_thread(WASMExecEnv *exec_env)
 {
+    os_mutex_lock(&global_exec_env_list_lock);
+
     if (!clusters_have_exec_env(exec_env) || !exec_env->cluster) {
         /* Invalid thread or the thread has exited */
+        os_mutex_unlock(&global_exec_env_list_lock);
         return 0;
     }
 
     set_thread_cancel_flags(exec_env);
 
+    os_mutex_unlock(&global_exec_env_list_lock);
+
     return 0;
 }
 
@@ -1559,7 +1580,7 @@ wasm_cluster_suspend_thread(WASMExecEnv *exec_env, WASMExecEnv *self)
     os_mutex_lock(&cluster->thread_state_lock);
     exec_env->suspend_count++;
     /* Set the suspend flag */
-    exec_env->suspend_flags.flags |= WASM_SUSPEND_FLAG_SUSPEND;
+    SUSPEND_FLAGS_SET(exec_env, SUSPEND);
     os_mutex_unlock(&cluster->thread_state_lock);
 
     wait_for_thread_suspend(exec_env);
@@ -1585,7 +1606,7 @@ wasm_cluster_suspend_all_except_self(WASMCluster *cluster, WASMExecEnv *self)
     while (exec_env) {
         if (exec_env != self) {
             exec_env->suspend_count++;
-            exec_env->suspend_flags.flags |= WASM_SUSPEND_FLAG_SUSPEND;
+            SUSPEND_FLAGS_SET(exec_env, SUSPEND);
         }
         exec_env = bh_list_elem_next(exec_env);
     }
@@ -1622,7 +1643,7 @@ wasm_cluster_resume_thread(WASMExecEnv *exec_env)
         exec_env->suspend_count--;
 
         if (exec_env->suspend_count == 0) {
-            exec_env->suspend_flags.flags &= ~WASM_SUSPEND_FLAG_SUSPEND;
+            SUSPEND_FLAGS_CLR(exec_env, SUSPEND);
             os_cond_broadcast(&cluster->thread_resume_cond);
         }
     }
@@ -1647,7 +1668,7 @@ wasm_cluster_resume_all_except_self(WASMCluster *cluster, WASMExecEnv *self)
             exec_env->suspend_count--;
 
             if (exec_env->suspend_count == 0) {
-                exec_env->suspend_flags.flags &= ~WASM_SUSPEND_FLAG_SUSPEND;
+                SUSPEND_FLAGS_CLR(exec_env, SUSPEND);
                 os_cond_broadcast(&cluster->thread_resume_cond);
             }
         }
@@ -1823,16 +1844,19 @@ wasm_cluster_is_thread_terminated(WASMExecEnv *exec_env)
 {
     bool is_thread_terminated;
 
+    os_mutex_lock(&global_exec_env_list_lock);
+
     if (!clusters_have_exec_env(exec_env)) {
+        os_mutex_unlock(&global_exec_env_list_lock);
         return true;
     }
 
     os_mutex_lock(&exec_env->cluster->thread_state_lock);
-    is_thread_terminated =
-        (exec_env->suspend_flags.flags & WASM_SUSPEND_FLAG_TERMINATE) ? true
-                                                                      : false;
+    is_thread_terminated = SUSPEND_FLAGS_ISSET(exec_env, TERMINATE);
     os_mutex_unlock(&exec_env->cluster->thread_state_lock);
 
+    os_mutex_unlock(&global_exec_env_list_lock);
+
     return is_thread_terminated;
 }