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

Raising "wasi proc exit" exception, spreading it to other threads and then
clearing it in all threads may result in unexpected behavior: the sub thread
may end first, handle the "wasi proc exit" exception and clear exceptions
of other threads, including the main thread. And when main thread's
exception is cleared, it may continue to run and throw "unreachable"
exception. This also leads to some assertion failed.

Ignore exception spreading for "wasi proc exit" and don't clear exception
of other threads to resolve the issue.

And add suspend flag check after atomic wait since the atomic wait may
be notified by other thread when exception occurs.
This commit is contained in:
Wenyong Huang 2023-02-24 20:05:39 +08:00 committed by GitHub
parent 63273a1673
commit 38c67b3f48
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 85 additions and 20 deletions

View File

@ -1880,8 +1880,10 @@ clear_wasi_proc_exit_exception(WASMModuleInstanceCommon *module_inst_comm)
if (exception && !strcmp(exception, "Exception: wasi proc exit")) { if (exception && !strcmp(exception, "Exception: wasi proc exit")) {
/* The "wasi proc exit" exception is thrown by native lib to /* The "wasi proc exit" exception is thrown by native lib to
let wasm app exit, which is a normal behavior, we clear let wasm app exit, which is a normal behavior, we clear
the exception here. */ the exception here. And just clear the exception of current
wasm_set_exception(module_inst, NULL); 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 true;
} }
return false; return false;

View File

@ -999,6 +999,14 @@ aot_compile_op_call(AOTCompContext *comp_ctx, AOTFuncContext *func_ctx,
} }
#endif #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; ret = true;
fail: fail:
if (param_types) if (param_types)
@ -1645,6 +1653,14 @@ aot_compile_op_call_indirect(AOTCompContext *comp_ctx, AOTFuncContext *func_ctx,
} }
#endif #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; ret = true;
fail: fail:

View File

@ -7,6 +7,7 @@
#include "aot_emit_exception.h" #include "aot_emit_exception.h"
#include "../aot/aot_runtime.h" #include "../aot/aot_runtime.h"
#include "aot_intrinsic.h" #include "aot_intrinsic.h"
#include "aot_emit_control.h"
#define BUILD_ICMP(op, left, right, res, name) \ #define BUILD_ICMP(op, left, right, res, name) \
do { \ do { \
@ -1344,7 +1345,7 @@ aot_compile_op_atomic_wait(AOTCompContext *comp_ctx, AOTFuncContext *func_ctx,
return false; 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_fail, "atomic_wait_fail");
ADD_BASIC_BLOCK(wait_success, "wait_success"); 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); 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; return true;
fail: fail:
return false; return false;

View File

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

View File

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

View File

@ -225,7 +225,9 @@ wasm_cluster_create(WASMExecEnv *exec_env)
/* Prepare the aux stack top and size for every thread */ /* Prepare the aux stack top and size for every thread */
if (!wasm_exec_env_get_aux_stack(exec_env, &aux_stack_start, if (!wasm_exec_env_get_aux_stack(exec_env, &aux_stack_start,
&aux_stack_size)) { &aux_stack_size)) {
#if WASM_ENABLE_LIB_WASI_THREADS == 0
LOG_VERBOSE("No aux stack info for this module, can't create thread"); 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, /* If the module don't have aux stack info, don't throw error here,
but remain stack_tops and stack_segment_occupied as NULL */ 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 *curr_wasm_inst =
(WASMModuleInstance *)get_module_inst(curr_exec_env); (WASMModuleInstance *)get_module_inst(curr_exec_env);
bh_memcpy_s(curr_wasm_inst->cur_exception, /* Only spread non "wasi proc exit" exception */
sizeof(curr_wasm_inst->cur_exception), if (!strstr(wasm_inst->cur_exception, "wasi proc exit")) {
wasm_inst->cur_exception, sizeof(wasm_inst->cur_exception)); 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 */ /* Terminate the thread so it can exit from dead loops */
set_thread_cancel_flags(curr_exec_env); set_thread_cancel_flags(curr_exec_env);
} }

View File

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

View File

@ -15,8 +15,14 @@
#include "wasi_thread_start.h" #include "wasi_thread_start.h"
#define TEST_TERMINATION_BY_TRAP 0 // Otherwise test `proc_exit` termination #define BUSY_WAIT 0
#define TEST_TERMINATION_IN_MAIN_THREAD 1 #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 TIMEOUT_SECONDS 10
#define NUM_THREADS 3 #define NUM_THREADS 3
@ -30,23 +36,28 @@ typedef struct {
void void
run_long_task() 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++) for (int i = 0; i < TIMEOUT_SECONDS; i++)
sleep(1); sleep(1);
#elif LONG_TASK_IMPL == ATOMIC_WAIT
__builtin_wasm_memory_atomic_wait32(0, 0, -1);
#else
sleep(TIMEOUT_SECONDS);
#endif
} }
void void
start_job() start_job()
{ {
sem_post(&sem); sem_post(&sem);
run_long_task(); // Wait to be interrupted run_long_task(); /* Wait to be interrupted */
assert(false && "Unreachable"); assert(false && "Unreachable");
} }
void void
terminate_process() 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"); printf("Waiting before terminating\n");
for (int i = 0; i < NUM_THREADS; i++) for (int i = 0; i < NUM_THREADS; i++)
sem_wait(&sem); sem_wait(&sem);
@ -55,7 +66,7 @@ terminate_process()
#if TEST_TERMINATION_BY_TRAP == 1 #if TEST_TERMINATION_BY_TRAP == 1
__builtin_trap(); __builtin_trap();
#else #else
__wasi_proc_exit(1); __wasi_proc_exit(33);
#endif #endif
} }
@ -86,14 +97,14 @@ main(int argc, char **argv)
} }
for (i = 0; i < NUM_THREADS; i++) { 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)) { if (!start_args_init(&data[i].base)) {
printf("Failed to allocate thread's stack\n"); printf("Failed to allocate thread's stack\n");
return EXIT_FAILURE; 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 #if TEST_TERMINATION_IN_MAIN_THREAD == 1
data[0].throw_exception = false; data[0].throw_exception = false;
#else #else
@ -105,7 +116,7 @@ main(int argc, char **argv)
return EXIT_FAILURE; return EXIT_FAILURE;
} }
// Create two additional threads to test exception propagation /* Create two additional threads to test exception propagation */
data[1].throw_exception = false; data[1].throw_exception = false;
thread_id = __wasi_thread_spawn(&data[1]); thread_id = __wasi_thread_spawn(&data[1]);
if (thread_id < 0) { if (thread_id < 0) {