Merge pull request #715 from bytecodealliance/main

thread-mgr: Fix spread "wasi proc exit" exception and atomic.wait issues (bytecodealliance#1988)

Author: wenyongh

core/iwasm/common/wasm_runtime_common.c

@@ -1880,8 +1880,10 @@ clear_wasi_proc_exit_exception(WASMModuleInstanceCommon *module_inst_comm)
     if (exception && !strcmp(exception, "Exception: wasi proc exit")) {
         /* The "wasi proc exit" exception is thrown by native lib to
            let wasm app exit, which is a normal behavior, we clear
-           the exception here. */
-        wasm_set_exception(module_inst, NULL);
+           the exception here. And just clear the exception of current
+           thread, don't call `wasm_set_exception(module_inst, NULL)`
+           which will clear the exception of all threads. */
+        module_inst->cur_exception[0] = '\0';
         return true;
     }
     return false;

core/iwasm/compilation/aot_emit_function.c

@@ -999,6 +999,14 @@ aot_compile_op_call(AOTCompContext *comp_ctx, AOTFuncContext *func_ctx,
     }
 #endif
 
+#if WASM_ENABLE_THREAD_MGR != 0
+    /* Insert suspend check point */
+    if (comp_ctx->enable_thread_mgr) {
+        if (!check_suspend_flags(comp_ctx, func_ctx))
+            goto fail;
+    }
+#endif
+
     ret = true;
 fail:
     if (param_types)
@@ -1645,6 +1653,14 @@ aot_compile_op_call_indirect(AOTCompContext *comp_ctx, AOTFuncContext *func_ctx,
     }
 #endif
 
+#if WASM_ENABLE_THREAD_MGR != 0
+    /* Insert suspend check point */
+    if (comp_ctx->enable_thread_mgr) {
+        if (!check_suspend_flags(comp_ctx, func_ctx))
+            goto fail;
+    }
+#endif
+
     ret = true;
 
 fail:

core/iwasm/compilation/aot_emit_memory.c

@@ -7,6 +7,7 @@
 #include "aot_emit_exception.h"
 #include "../aot/aot_runtime.h"
 #include "aot_intrinsic.h"
+#include "aot_emit_control.h"
 
 #define BUILD_ICMP(op, left, right, res, name)                                \
     do {                                                                      \
@@ -1344,7 +1345,7 @@ aot_compile_op_atomic_wait(AOTCompContext *comp_ctx, AOTFuncContext *func_ctx,
         return false;
     }
 
-    BUILD_ICMP(LLVMIntSGT, ret_value, I32_ZERO, cmp, "atomic_wait_ret");
+    BUILD_ICMP(LLVMIntNE, ret_value, I32_NEG_ONE, cmp, "atomic_wait_ret");
 
     ADD_BASIC_BLOCK(wait_fail, "atomic_wait_fail");
     ADD_BASIC_BLOCK(wait_success, "wait_success");
@@ -1368,6 +1369,14 @@ aot_compile_op_atomic_wait(AOTCompContext *comp_ctx, AOTFuncContext *func_ctx,
 
     PUSH_I32(ret_value);
 
+#if WASM_ENABLE_THREAD_MGR != 0
+    /* Insert suspend check point */
+    if (comp_ctx->enable_thread_mgr) {
+        if (!check_suspend_flags(comp_ctx, func_ctx))
+            return false;
+    }
+#endif
+
     return true;
 fail:
     return false;

core/iwasm/interpreter/wasm_interp_classic.c

@@ -3422,6 +3422,10 @@ wasm_interp_call_func_bytecode(WASMModuleInstance *module,
                         if (ret == (uint32)-1)
                             goto got_exception;
 
+#if WASM_ENABLE_THREAD_MGR != 0
+                        CHECK_SUSPEND_FLAGS();
+#endif
+
                         PUSH_I32(ret);
                         break;
                     }
@@ -3442,6 +3446,10 @@ wasm_interp_call_func_bytecode(WASMModuleInstance *module,
                         if (ret == (uint32)-1)
                             goto got_exception;
 
+#if WASM_ENABLE_THREAD_MGR != 0
+                        CHECK_SUSPEND_FLAGS();
+#endif
+
                         PUSH_I32(ret);
                         break;
                     }
@@ -3894,10 +3902,10 @@ wasm_interp_call_func_bytecode(WASMModuleInstance *module,
             PUSH_CSP(LABEL_TYPE_FUNCTION, 0, cell_num, frame_ip_end - 1);
 
             wasm_exec_env_set_cur_frame(exec_env, frame);
+        }
 #if WASM_ENABLE_THREAD_MGR != 0
-            CHECK_SUSPEND_FLAGS();
+        CHECK_SUSPEND_FLAGS();
 #endif
-        }
         HANDLE_OP_END();
     }
 

core/iwasm/interpreter/wasm_interp_fast.c

@@ -3266,6 +3266,10 @@ wasm_interp_call_func_bytecode(WASMModuleInstance *module,
                         if (ret == (uint32)-1)
                             goto got_exception;
 
+#if WASM_ENABLE_THREAD_MGR != 0
+                        CHECK_SUSPEND_FLAGS();
+#endif
+
                         PUSH_I32(ret);
                         break;
                     }
@@ -3286,6 +3290,10 @@ wasm_interp_call_func_bytecode(WASMModuleInstance *module,
                         if (ret == (uint32)-1)
                             goto got_exception;
 
+#if WASM_ENABLE_THREAD_MGR != 0
+                        CHECK_SUSPEND_FLAGS();
+#endif
+
                         PUSH_I32(ret);
                         break;
                     }
@@ -3826,6 +3834,9 @@ wasm_interp_call_func_bytecode(WASMModuleInstance *module,
 
             wasm_exec_env_set_cur_frame(exec_env, (WASMRuntimeFrame *)frame);
         }
+#if WASM_ENABLE_THREAD_MGR != 0
+        CHECK_SUSPEND_FLAGS();
+#endif
         HANDLE_OP_END();
     }
 

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

@@ -225,7 +225,9 @@ wasm_cluster_create(WASMExecEnv *exec_env)
     /* Prepare the aux stack top and size for every thread */
     if (!wasm_exec_env_get_aux_stack(exec_env, &aux_stack_start,
                                      &aux_stack_size)) {
+#if WASM_ENABLE_LIB_WASI_THREADS == 0
         LOG_VERBOSE("No aux stack info for this module, can't create thread");
+#endif
 
         /* If the module don't have aux stack info, don't throw error here,
             but remain stack_tops and stack_segment_occupied as NULL */
@@ -1131,9 +1133,14 @@ set_exception_visitor(void *node, void *user_data)
         WASMModuleInstance *curr_wasm_inst =
             (WASMModuleInstance *)get_module_inst(curr_exec_env);
 
-        bh_memcpy_s(curr_wasm_inst->cur_exception,
-                    sizeof(curr_wasm_inst->cur_exception),
-                    wasm_inst->cur_exception, sizeof(wasm_inst->cur_exception));
+        /* Only spread non "wasi proc exit" exception */
+        if (!strstr(wasm_inst->cur_exception, "wasi proc exit")) {
+            bh_memcpy_s(curr_wasm_inst->cur_exception,
+                        sizeof(curr_wasm_inst->cur_exception),
+                        wasm_inst->cur_exception,
+                        sizeof(wasm_inst->cur_exception));
+        }
+
         /* Terminate the thread so it can exit from dead loops */
         set_thread_cancel_flags(curr_exec_env);
     }
@@ -1203,4 +1210,4 @@ bool
 wasm_cluster_is_thread_terminated(WASMExecEnv *exec_env)
 {
     return (exec_env->suspend_flags.flags & 0x01) ? true : false;
-}
+}

samples/wasi-threads/README.md

@@ -26,6 +26,7 @@ $ ./iwasm wasm-apps/exception_propagation.wasm
 ## Run samples in AOT mode
 ```shell
 $ ../../../wamr-compiler/build/wamrc \
+    --enable-multi-thread \
     -o wasm-apps/no_pthread.aot wasm-apps/no_pthread.wasm
 $ ./iwasm wasm-apps/no_pthread.aot
 ```

samples/wasi-threads/wasm-apps/thread_termination.c

@@ -15,8 +15,14 @@
 
 #include "wasi_thread_start.h"
 
-#define TEST_TERMINATION_BY_TRAP 0 // Otherwise test `proc_exit` termination
-#define TEST_TERMINATION_IN_MAIN_THREAD 1
+#define BUSY_WAIT 0
+#define ATOMIC_WAIT 1
+#define POLL_ONEOFF 2
+
+/* Change parameters here to modify the sample behavior */
+#define TEST_TERMINATION_BY_TRAP 0 /* Otherwise `proc_exit` termination */
+#define TEST_TERMINATION_IN_MAIN_THREAD 1 /* Otherwise in spawn thread */
+#define LONG_TASK_IMPL ATOMIC_WAIT
 
 #define TIMEOUT_SECONDS 10
 #define NUM_THREADS 3
@@ -30,23 +36,28 @@ typedef struct {
 void
 run_long_task()
 {
-    // Busy waiting to be interruptible by trap or `proc_exit`
+#if LONG_TASK_IMPL == BUSY_WAIT
     for (int i = 0; i < TIMEOUT_SECONDS; i++)
         sleep(1);
+#elif LONG_TASK_IMPL == ATOMIC_WAIT
+    __builtin_wasm_memory_atomic_wait32(0, 0, -1);
+#else
+    sleep(TIMEOUT_SECONDS);
+#endif
 }
 
 void
 start_job()
 {
     sem_post(&sem);
-    run_long_task(); // Wait to be interrupted
+    run_long_task(); /* Wait to be interrupted */
     assert(false && "Unreachable");
 }
 
 void
 terminate_process()
 {
-    // Wait for all other threads (including main thread) to be ready
+    /* Wait for all other threads (including main thread) to be ready */
     printf("Waiting before terminating\n");
     for (int i = 0; i < NUM_THREADS; i++)
         sem_wait(&sem);
@@ -55,7 +66,7 @@ terminate_process()
 #if TEST_TERMINATION_BY_TRAP == 1
     __builtin_trap();
 #else
-    __wasi_proc_exit(1);
+    __wasi_proc_exit(33);
 #endif
 }
 
@@ -86,14 +97,14 @@ main(int argc, char **argv)
     }
 
     for (i = 0; i < NUM_THREADS; i++) {
-        // No graceful memory free to simplify the example
+        /* No graceful memory free to simplify the example */
         if (!start_args_init(&data[i].base)) {
             printf("Failed to allocate thread's stack\n");
             return EXIT_FAILURE;
         }
     }
 
-// Create a thread that forces termination through trap or `proc_exit`
+    /* Create a thread that forces termination through trap or `proc_exit` */
 #if TEST_TERMINATION_IN_MAIN_THREAD == 1
     data[0].throw_exception = false;
 #else
@@ -105,7 +116,7 @@ main(int argc, char **argv)
         return EXIT_FAILURE;
     }
 
-    // Create two additional threads to test exception propagation
+    /* Create two additional threads to test exception propagation */
     data[1].throw_exception = false;
     thread_id = __wasi_thread_spawn(&data[1]);
     if (thread_id < 0) {
@@ -128,4 +139,4 @@ main(int argc, char **argv)
     start_job();
 #endif /* TEST_TERMINATION_IN_MAIN_THREAD */
     return EXIT_SUCCESS;
-}
+}