Skip to content

Allow interrupting blocking instructions #1930

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 5 commits into from
Closed

Allow interrupting blocking instructions #1930

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3c2f060
fix(wasi-threads): interrupt blocking instructions using signals
eloparco Feb 2, 2023
185cd4d
fix(wasi-threads): fix platform includes
eloparco Feb 3, 2023
f9c0bd1
refactor(wasi-threads): move platform-related code and remove "block …
eloparco Feb 4, 2023
6eecad7
feat(wasi-threads): use blocking operations in thread termination exa…
eloparco Feb 5, 2023
9d6dc29
fix(wasi-threads): handle signal handler being called before os_setjmp
eloparco Feb 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions build-scripts/config_common.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,10 @@ else ()
add_definitions (-DWASM_DISABLE_STACK_HW_BOUND_CHECK=0)
endif ()
endif ()
if (WAMR_DISABLE_BLOCK_INSN_INTERRUPT EQUAL 1)
add_definitions (-DWASM_DISABLE_BLOCK_INSN_INTERRUPT=1)
message (" Interruption of blocking instructions disabled")
endif ()
if (WAMR_BUILD_SIMD EQUAL 1)
if (NOT WAMR_BUILD_TARGET MATCHES "RISCV64.*")
add_definitions (-DWASM_ENABLE_SIMD=1)
Expand Down
7 changes: 7 additions & 0 deletions core/config.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,13 @@
#define WASM_DISABLE_STACK_HW_BOUND_CHECK 0
#endif

/* Disable the capability of interrupting blocking instructions
* (e.g. atomic.wait, sleep) when an exception (e.g. trap, proc_exit) is raised
*/
#ifndef WASM_DISABLE_BLOCK_INSN_INTERRUPT
#define WASM_DISABLE_BLOCK_INSN_INTERRUPT 0
#endif

/* Disable SIMD unless it is manualy enabled somewhere */
#ifndef WASM_ENABLE_SIMD
#define WASM_ENABLE_SIMD 0
Expand Down
48 changes: 45 additions & 3 deletions core/iwasm/aot/aot_runtime.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1310,11 +1310,53 @@ invoke_native_with_hw_bound_check(WASMExecEnv *exec_env, void *func_ptr,
(void)jmpbuf_node_pop;
return ret;
}
#endif /* end of OS_ENABLE_HW_BOUND_CHECK */

#ifndef OS_ENABLE_BLOCK_INSN_INTERRUPT

#ifdef OS_ENABLE_HW_BOUND_CHECK
#define invoke_native_internal invoke_native_with_hw_bound_check
#else /* else of OS_ENABLE_HW_BOUND_CHECK */
#else
#define invoke_native_internal wasm_runtime_invoke_native
#endif /* end of OS_ENABLE_HW_BOUND_CHECK */
#endif

#else /* else of OS_ENABLE_BLOCK_INSN_INTERRUPT */

#ifdef OS_ENABLE_HW_BOUND_CHECK
#define invoke_native_block_insn_interrupt invoke_native_with_hw_bound_check
#else
#define invoke_native_block_insn_interrupt wasm_runtime_invoke_native
#endif

static bool
invoke_native_internal(WASMExecEnv *exec_env, void *func_ptr,
const WASMType *func_type, const char *signature,
void *attachment, uint32 *argv, uint32 argc,
uint32 *argv_ret)
{
int ret = false;
WASMJmpBuf jmpbuf_node = { 0 }, *jmpbuf_node_pop;
wasm_runtime_set_exec_env_tls(exec_env);

if (os_setjmp(jmpbuf_node.jmpbuf) == 0) {
wasm_exec_env_push_jmpbuf(exec_env, &jmpbuf_node);
ret = invoke_native_block_insn_interrupt(exec_env, func_ptr, func_type,
signature, attachment, argv,
argc, argv_ret);
}
else {
ret = false;
}

jmpbuf_node_pop = wasm_exec_env_pop_jmpbuf(exec_env);
bh_assert(&jmpbuf_node == jmpbuf_node_pop);
if (!exec_env->jmpbuf_stack_top) {
wasm_runtime_set_exec_env_tls(NULL);
}

return ret;
}
#endif /* end of OS_ENABLE_BLOCK_INSN_INTERRUPT */

bool
aot_call_function(WASMExecEnv *exec_env, AOTFunctionInstance *function,
Expand Down Expand Up @@ -1471,7 +1513,7 @@ aot_create_exec_env_and_call_function(AOTModuleInstance *module_inst,
WASMExecEnv *exec_env = NULL, *existing_exec_env = NULL;
bool ret;

#if defined(OS_ENABLE_HW_BOUND_CHECK)
#if defined(OS_ENABLE_HW_BOUND_CHECK) || defined(OS_ENABLE_BLOCK_INSN_INTERRUPT)
existing_exec_env = exec_env = wasm_runtime_get_exec_env_tls();
#elif WASM_ENABLE_THREAD_MGR != 0
existing_exec_env = exec_env =
Expand Down
2 changes: 1 addition & 1 deletion core/iwasm/common/wasm_exec_env.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ wasm_exec_env_set_thread_arg(WASMExecEnv *exec_env, void *thread_arg)
}
#endif

#ifdef OS_ENABLE_HW_BOUND_CHECK
#if defined(OS_ENABLE_HW_BOUND_CHECK) || defined(OS_ENABLE_BLOCK_INSN_INTERRUPT)
void
wasm_exec_env_push_jmpbuf(WASMExecEnv *exec_env, WASMJmpBuf *jmpbuf)
{
Expand Down
8 changes: 5 additions & 3 deletions core/iwasm/common/wasm_exec_env.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ typedef struct WASMCurrentEnvStatus WASMCurrentEnvStatus;
#endif
#endif

#ifdef OS_ENABLE_HW_BOUND_CHECK
#if defined(OS_ENABLE_HW_BOUND_CHECK) || defined(OS_ENABLE_BLOCK_INSN_INTERRUPT)
typedef struct WASMJmpBuf {
struct WASMJmpBuf *prev;
korp_jmpbuf jmpbuf;
Expand Down Expand Up @@ -135,8 +135,10 @@ typedef struct WASMExecEnv {
BlockAddr block_addr_cache[BLOCK_ADDR_CACHE_SIZE][BLOCK_ADDR_CONFLICT_SIZE];
#endif

#ifdef OS_ENABLE_HW_BOUND_CHECK
#if defined(OS_ENABLE_HW_BOUND_CHECK) || defined(OS_ENABLE_BLOCK_INSN_INTERRUPT)
WASMJmpBuf *jmpbuf_stack_top;
#endif
#ifdef OS_ENABLE_HW_BOUND_CHECK
/* One guard page for the exception check */
uint8 *exce_check_guard_page;
#endif
Expand Down Expand Up @@ -291,7 +293,7 @@ void
wasm_exec_env_set_thread_arg(WASMExecEnv *exec_env, void *thread_arg);
#endif

#ifdef OS_ENABLE_HW_BOUND_CHECK
#if defined(OS_ENABLE_HW_BOUND_CHECK) || defined(OS_ENABLE_BLOCK_INSN_INTERRUPT)
void
wasm_exec_env_push_jmpbuf(WASMExecEnv *exec_env, WASMJmpBuf *jmpbuf);

Expand Down
29 changes: 26 additions & 3 deletions core/iwasm/common/wasm_runtime_common.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,12 +128,14 @@ runtime_malloc(uint64 size, WASMModuleInstanceCommon *module_inst,
static JitCompOptions jit_options = { 0 };
#endif

#ifdef OS_ENABLE_HW_BOUND_CHECK
#if defined(OS_ENABLE_HW_BOUND_CHECK) || defined(OS_ENABLE_BLOCK_INSN_INTERRUPT)
/* The exec_env of thread local storage, set before calling function
and used in signal handler, as we cannot get it from the argument
of signal handler */
static os_thread_local_attribute WASMExecEnv *exec_env_tls = NULL;
#endif

#ifdef OS_ENABLE_HW_BOUND_CHECK
#ifndef BH_PLATFORM_WINDOWS
static void
runtime_signal_handler(void *sig_addr)
Expand Down Expand Up @@ -303,7 +305,9 @@ runtime_signal_destroy()
#endif
os_thread_signal_destroy();
}
#endif /* end of OS_ENABLE_HW_BOUND_CHECK */

#if defined(OS_ENABLE_HW_BOUND_CHECK) || defined(OS_ENABLE_BLOCK_INSN_INTERRUPT)
void
wasm_runtime_set_exec_env_tls(WASMExecEnv *exec_env)
{
Expand All @@ -315,7 +319,20 @@ wasm_runtime_get_exec_env_tls()
{
return exec_env_tls;
}
#endif /* end of OS_ENABLE_HW_BOUND_CHECK */
#endif

#ifdef OS_ENABLE_BLOCK_INSN_INTERRUPT
static void
interrupt_block_insn_sig_handler()
{
WASMJmpBuf *jmpbuf_node = exec_env_tls->jmpbuf_stack_top;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jmpbuf_stack_top here can be for another handler.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, if HW bound check is enabled, it will get a "HW bound" jmpbuf #1930 (comment), but that should not be a problem unless I'm missing something

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can even get a signal between wasm_exec_env_push_jmpbuf and os_setjmp.

Copy link
Contributor Author

@eloparco eloparco Feb 6, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can even get a signal between wasm_exec_env_push_jmpbuf and os_setjmp.

In that case, the jmpbuf would be invalid, as it is in general before being set up by os_setjmp. Isn't that the same for the HW bound check before my PR?

I wonder if we have to mask/un-mask signals to avoid the signal handler being called in the middle. We want to avoid signal handlers being called between wasm_exec_env_push_jmpbuf and os_setjmp as they would use a non-initialized jmpbuf iiuc.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it isn't a problem for hw bound check because the signal for it is basically synchronous to the thread execution. ie. the thread itself can control where it can happen.

Copy link
Contributor Author

@eloparco eloparco Feb 7, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we maybe invert the order of wasm_exec_env_push_jmpbuf and os_setjmp? So that the jmpbuf is only pushed after being initialized by os_setjmp.

But there are probably other cases that are not covered, like an exception before os_setjmp. [EDIT] These cases shouldn't be a problem since they're already handled by the normal exception spreading mechanism.

Copy link
Contributor Author

@eloparco eloparco Feb 7, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we maybe invert the order of wasm_exec_env_push_jmpbuf and os_setjmp? So that the jmpbuf is only pushed after being initialized by os_setjmp.

I tried that in the latest commit, but maybe it's safer to use something like static volatile sig_atomic_t canjump; (but with tls) as explained here https://notes.shichao.io/apue/ch10/?

you can even get a signal between wasm_exec_env_push_jmpbuf and os_setjmp.

Apart from this, do you think there are other cases that are not covered? If we get the signal before os_setjmp, the signal handler (for instruction interruption) will return and the thread will stop when starting to execute instructions, using the normal exception propagation mechanism.

if (!jmpbuf_node) {
return;
}

os_longjmp(jmpbuf_node->jmpbuf, 1);
}
#endif /* OS_ENABLE_BLOCK_INSN_INTERRUPT */

static bool
wasm_runtime_env_init()
Expand Down Expand Up @@ -348,7 +365,11 @@ wasm_runtime_env_init()
goto fail5;
}
#endif

#ifdef OS_ENABLE_BLOCK_INSN_INTERRUPT
if (!os_interrupt_block_insn_init(interrupt_block_insn_sig_handler)) {
goto fail6;
}
#endif
#ifdef OS_ENABLE_HW_BOUND_CHECK
if (!runtime_signal_init()) {
goto fail6;
Expand Down Expand Up @@ -404,8 +425,10 @@ wasm_runtime_env_init()
fail7:
#endif
#endif
#if defined(OS_ENABLE_HW_BOUND_CHECK) || defined(OS_ENABLE_BLOCK_INSN_INTERRUPT)
#ifdef OS_ENABLE_HW_BOUND_CHECK
runtime_signal_destroy();
#endif
fail6:
#endif
#if (WASM_ENABLE_WAMR_COMPILER == 0) && (WASM_ENABLE_THREAD_MGR != 0)
Expand Down
2 changes: 2 additions & 0 deletions core/iwasm/common/wasm_runtime_common.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -423,7 +423,9 @@ typedef struct WASMSignalInfo {
EXCEPTION_POINTERS *exce_info;
#endif
} WASMSignalInfo;
#endif

#if defined(OS_ENABLE_HW_BOUND_CHECK) || defined(OS_ENABLE_BLOCK_INSN_INTERRUPT)
/* Set exec_env of thread local storage */
void
wasm_runtime_set_exec_env_tls(WASMExecEnv *exec_env);
Expand Down
37 changes: 36 additions & 1 deletion core/iwasm/interpreter/wasm_runtime.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2126,11 +2126,45 @@ call_wasm_with_hw_bound_check(WASMModuleInstance *module_inst,
}
(void)jmpbuf_node_pop;
}
#endif /* end of OS_ENABLE_HW_BOUND_CHECK */

#ifndef OS_ENABLE_BLOCK_INSN_INTERRUPT

#ifdef OS_ENABLE_HW_BOUND_CHECK
#define interp_call_wasm call_wasm_with_hw_bound_check
#else
#define interp_call_wasm wasm_interp_call_wasm
#endif

#else /* else of OS_ENABLE_BLOCK_INSN_INTERRUPT */

#ifdef OS_ENABLE_HW_BOUND_CHECK
#define call_wasm_block_insn_interrupt call_wasm_with_hw_bound_check
#else
#define call_wasm_block_insn_interrupt wasm_interp_call_wasm
#endif

static void
interp_call_wasm(WASMModuleInstance *module_inst, WASMExecEnv *exec_env,
WASMFunctionInstance *function, unsigned argc, uint32 argv[])
{
WASMJmpBuf jmpbuf_node = { 0 }, *jmpbuf_node_pop;
wasm_runtime_set_exec_env_tls(exec_env);

if (os_setjmp(jmpbuf_node.jmpbuf) == 0) {
wasm_exec_env_push_jmpbuf(exec_env, &jmpbuf_node);
call_wasm_block_insn_interrupt(module_inst, exec_env, function, argc,
argv);
}

jmpbuf_node_pop = wasm_exec_env_pop_jmpbuf(exec_env);
bh_assert(&jmpbuf_node == jmpbuf_node_pop);
if (!exec_env->jmpbuf_stack_top) {
wasm_runtime_set_exec_env_tls(NULL);
}
}
#endif /* end of OS_ENABLE_BLOCK_INSN_INTERRUPT */

bool
wasm_call_function(WASMExecEnv *exec_env, WASMFunctionInstance *function,
unsigned argc, uint32 argv[])
Expand All @@ -2142,6 +2176,7 @@ wasm_call_function(WASMExecEnv *exec_env, WASMFunctionInstance *function,
wasm_exec_env_set_thread_info(exec_env);

interp_call_wasm(module_inst, exec_env, function, argc, argv);

return !wasm_get_exception(module_inst) ? true : false;
}

Expand All @@ -2153,7 +2188,7 @@ wasm_create_exec_env_and_call_function(WASMModuleInstance *module_inst,
WASMExecEnv *exec_env = NULL, *existing_exec_env = NULL;
bool ret;

#if defined(OS_ENABLE_HW_BOUND_CHECK)
#if defined(OS_ENABLE_HW_BOUND_CHECK) || defined(OS_ENABLE_BLOCK_INSN_INTERRUPT)
existing_exec_env = exec_env = wasm_runtime_get_exec_env_tls();
#elif WASM_ENABLE_THREAD_MGR != 0
existing_exec_env = exec_env =
Expand Down
10 changes: 8 additions & 2 deletions core/iwasm/libraries/thread-mgr/thread_manager.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -520,7 +520,7 @@ thread_manager_start_routine(void *arg)
exec_env->handle = os_self_thread();
ret = exec_env->thread_start_routine(exec_env);

#ifdef OS_ENABLE_HW_BOUND_CHECK
#if defined(OS_ENABLE_HW_BOUND_CHECK) || defined(OS_ENABLE_BLOCK_INSN_INTERRUPT)
if (exec_env->suspend_flags.flags & 0x08)
ret = exec_env->thread_ret_value;
#endif
Expand Down Expand Up @@ -826,7 +826,7 @@ wasm_cluster_exit_thread(WASMExecEnv *exec_env, void *retval)
{
WASMCluster *cluster;

#ifdef OS_ENABLE_HW_BOUND_CHECK
#if defined(OS_ENABLE_HW_BOUND_CHECK) || defined(OS_ENABLE_BLOCK_INSN_INTERRUPT)
if (exec_env->jmpbuf_stack_top) {
/* Store the return value in exec_env */
exec_env->thread_ret_value = retval;
Expand Down Expand Up @@ -1064,6 +1064,12 @@ set_exception_visitor(void *node, void *user_data)
bh_memcpy_s(curr_wasm_inst->cur_exception,
sizeof(curr_wasm_inst->cur_exception),
wasm_inst->cur_exception, sizeof(wasm_inst->cur_exception));

#ifdef OS_ENABLE_BLOCK_INSN_INTERRUPT
bh_assert(curr_exec_env->handle);
os_thread_kill(curr_exec_env->handle);
#endif

/* Terminate the thread so it can exit from dead loops */
set_thread_cancel_flags(curr_exec_env);
}
Expand Down
21 changes: 15 additions & 6 deletions core/shared/platform/android/platform_internal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,19 +63,21 @@ typedef sem_t korp_sem;

#define bh_socket_t int

#if WASM_DISABLE_BLOCK_INSN_INTERRUPT == 0 && WASM_ENABLE_THREAD_MGR != 0
#define OS_ENABLE_BLOCK_INSN_INTERRUPT

typedef void (*os_block_insn_sig_handler)();
bool
os_interrupt_block_insn_init(os_block_insn_sig_handler handler);
#endif

#if WASM_DISABLE_HW_BOUND_CHECK == 0
#if defined(BUILD_TARGET_X86_64) || defined(BUILD_TARGET_AMD_64) \
|| defined(BUILD_TARGET_AARCH64) || defined(BUILD_TARGET_RISCV64_LP64D) \
|| defined(BUILD_TARGET_RISCV64_LP64)

#include <setjmp.h>

#define OS_ENABLE_HW_BOUND_CHECK

typedef jmp_buf korp_jmpbuf;

#define os_setjmp setjmp
#define os_longjmp longjmp
#define os_alloca alloca

#define os_getpagesize getpagesize
Expand All @@ -99,6 +101,13 @@ os_sigreturn();
#endif /* end of BUILD_TARGET_X86_64/AMD_64/AARCH64/RISCV64 */
#endif /* end of WASM_DISABLE_HW_BOUND_CHECK */

#if defined(OS_ENABLE_BLOCK_INSN_INTERRUPT) || defined(OS_ENABLE_HW_BOUND_CHECK)
#include <setjmp.h>
typedef jmp_buf korp_jmpbuf;
#define os_setjmp setjmp
#define os_longjmp longjmp
#endif

typedef long int __syscall_slong_t;

#if __ANDROID_API__ < 19
Expand Down
23 changes: 23 additions & 0 deletions core/shared/platform/common/posix/posix_thread.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,12 @@ os_thread_exit(void *retval)
return pthread_exit(retval);
}

int
os_thread_kill(korp_tid tid)
{
return pthread_kill(tid, SIGUSR1);
}

#if defined(os_thread_local_attribute)
static os_thread_local_attribute uint8 *thread_stack_boundary = NULL;
#endif
Expand Down Expand Up @@ -414,6 +420,23 @@ os_thread_get_stack_boundary()
return addr;
}

#ifdef OS_ENABLE_BLOCK_INSN_INTERRUPT
bool
os_interrupt_block_insn_init(os_block_insn_sig_handler handler)
{
struct sigaction act;
memset(&act, 0, sizeof(act));
act.sa_handler = handler;
sigfillset(&act.sa_mask);
if (sigaction(SIGUSR1, &act, NULL) < 0) {
os_printf("failed to set signal handler\n");
return false;
}

return true;
}
#endif /* OS_ENABLE_BLOCK_INSN_INTERRUPT */

#ifdef OS_ENABLE_HW_BOUND_CHECK

#define SIG_ALT_STACK_SIZE (32 * 1024)
Expand Down
Loading