Interrupt blocking instructions #1921
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,6 +9,13 @@ | |
| #include "platform_api_vmcore.h" | ||
| #include "platform_api_extension.h" | ||
|
|
||
| #include "bh_hashmap.h" | ||
| #include <setjmp.h> | ||
|
|
||
| #define STACK_CTX_SIG SIGUSR1 | ||
| static os_thread_local_attribute sigset_t newmask, oldmask; | ||
| bh_stack_context_handler_t stack_context_handler; | ||
|
|
||
| typedef struct { | ||
| thread_start_routine_t start; | ||
| void *arg; | ||
|
|
@@ -47,6 +54,164 @@ os_thread_wrapper(void *arg) | |
| return NULL; | ||
| } | ||
|
|
||
| static void | ||
| stack_context_sig_handler(int sig) | ||
| { | ||
| assert(sig == STACK_CTX_SIG); | ||
| korp_tid self = pthread_self(); | ||
|
|
||
| /* Get latest stack context (first in the list) for current thread */ | ||
| bh_list *context_list = bh_hash_map_find(stack_context_handler.contexts, | ||
| (void *)(uintptr_t)self); | ||
| assert(context_list); | ||
| sigjmp_buf *context = bh_list_first_elem(context_list); | ||
| assert(context); | ||
|
|
||
| siglongjmp(*context, 1); | ||
| } | ||
|
|
||
| sigjmp_buf * | ||
| os_create_stack_context(korp_tid handle) | ||
| { | ||
| return BH_MALLOC(sizeof(sigjmp_buf)); | ||
| } | ||
|
|
||
| static void | ||
| free_stack_context_list(void *stack_context_list) | ||
| { | ||
| bh_list *context_list = (bh_list *)stack_context_list; | ||
|
|
||
| sigjmp_buf *context = bh_list_first_elem(context_list); | ||
| while (context) { | ||
| sigjmp_buf *next = bh_list_elem_next(context); | ||
| BH_FREE(context); | ||
| context = next; | ||
| }; | ||
|
|
||
| BH_FREE(context_list); | ||
| } | ||
|
|
||
| static inline void | ||
| stack_ctx_block_sig_and_lock() | ||
| { | ||
| sigemptyset(&newmask); | ||
| sigaddset(&newmask, STACK_CTX_SIG); | ||
| pthread_sigmask(SIG_BLOCK, &newmask, &oldmask); | ||
|
|
||
| os_mutex_lock(&stack_context_handler.lock); | ||
| } | ||
|
|
||
| static inline void | ||
| stack_ctx_unlock_and_unblock_sig() | ||
| { | ||
| os_mutex_unlock(&stack_context_handler.lock); | ||
| pthread_sigmask(SIG_SETMASK, &oldmask, NULL); | ||
| } | ||
|
|
||
| bool | ||
| os_stack_contexts_init() | ||
| { | ||
| struct sigaction act; | ||
| memset(&act, 0, sizeof(act)); | ||
| act.sa_handler = stack_context_sig_handler; | ||
| sigfillset(&act.sa_mask); | ||
| if (sigaction(STACK_CTX_SIG, &act, NULL) < 0) { | ||
| LOG_ERROR("failed to set signal handler"); | ||
| return false; | ||
| } | ||
|
|
||
| if (os_mutex_init(&stack_context_handler.lock) != 0) { | ||
| LOG_ERROR("failed to init context mutex"); | ||
| return false; | ||
| } | ||
|
|
||
| stack_context_handler.contexts = bh_hash_map_create( | ||
| 32, false, (HashFunc)thread_handle_hash, | ||
| (KeyEqualFunc)thread_handle_equal, NULL, free_stack_context_list); | ||
| if (stack_context_handler.contexts == NULL) { | ||
| LOG_ERROR("failed to init stack contexts"); | ||
| return false; | ||
| } | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
| void | ||
| os_stack_contexts_destroy() | ||
| { | ||
| if (!bh_hash_map_destroy(stack_context_handler.contexts)) | ||
| LOG_ERROR("failed to destroy stack context map"); | ||
| stack_context_handler.contexts = NULL; | ||
|
|
||
| if (os_mutex_destroy(&stack_context_handler.lock) != BHT_OK) | ||
| LOG_ERROR("failed to destroy stack contexts"); | ||
| } | ||
|
|
||
| void | ||
| os_remove_stack_context(korp_tid handle, sigjmp_buf *context) | ||
| { | ||
| korp_tid self = pthread_self(); | ||
|
|
||
| stack_ctx_block_sig_and_lock(); | ||
|
There was a problem hiding this comment. why don't you make it thread-local? There was a problem hiding this comment. Good point, otherwise it would be shared between threads (and would then require access protection). Actually, I wanted to avoid globals and return the |
||
|
|
||
| bh_list *context_list = bh_hash_map_find(stack_context_handler.contexts, | ||
| (void *)(uintptr_t)self); | ||
| assert(context_list); | ||
| int ret = bh_list_remove(context_list, context); | ||
| assert(ret == BH_LIST_SUCCESS); | ||
| BH_FREE(context); | ||
|
|
||
| stack_ctx_unlock_and_unblock_sig(); | ||
| } | ||
|
|
||
| bool | ||
| os_is_returning_from_signal(korp_tid handle, sigjmp_buf *context) | ||
| { | ||
| if (sigsetjmp(*context, 1)) | ||
| return true; | ||
|
There was a problem hiding this comment. this looks unsafe to me. to make this work, os_is_returning_from_signal should always be inlined. There was a problem hiding this comment. thanks, that might explain some problems that I was seeing when trying to use it with AOT. In any case, I realized that I missed this PR #1309 that is basically doing something very similar to what I want to do. The approach is better because we don't need to keep a map <thread_id, stack contexts> if we use tls. I'll try to apply a similar approach. There was a problem hiding this comment. @eloparco Yes, in the memory access boundary check with hardware trap feature, both interpreter and AOT use the setjmp/longjmp mechanism in posix like platforms. The local variable jmpbuf is pushed into exec_env's jmpbuf stack before setjmp, runtime doesn't allocate memory for it. I think there may be two ways to implement this PR: one is to extend the hw bound check feature, e.g. reuse the current signal handler, the current setjmp and longjmp process, but add another macro to control them respectively (for example, OS_ENABLE_XXX and OS_ENABLE_HW_BOUND_CHECK). The other is to register new signal handler, add new setjmp/longjmp process, but I suggest not to add setjmp in wasm_interp_class.c, had better add in wasm_runtime.c, before calling wasm function: And note that setjmp is only supported by posix platforms, there should be macro to control the feature, and had better use os_setjmp/os_longjmp. There was a problem hiding this comment. And could you submit this PR to main branch since the main branch also has the issue? There was a problem hiding this comment. yes that works, for aot I'm not sure what to wrap with wasm-micro-runtime/core/iwasm/aot/aot_runtime.c Line 1411 in fe3347d wasm-micro-runtime/core/iwasm/aot/aot_runtime.c Line 1470 in fe3347d There was a problem hiding this comment.
I'll try to have it working here first since I'm testing it with wasi threads, then I'll create a PR in main to merge it there There was a problem hiding this comment. I'm closing this PR in favour of #1930, where I rely on what's already implemented for hardware bound checks. |
||
|
|
||
| stack_ctx_block_sig_and_lock(); | ||
|
|
||
| /* Get or create list of stack contexts for current thread */ | ||
| bh_list *context_list = bh_hash_map_find(stack_context_handler.contexts, | ||
| (void *)(uintptr_t)handle); | ||
| if (!context_list) { | ||
| context_list = BH_MALLOC(sizeof(bh_list)); | ||
| if (!context_list) { | ||
| LOG_ERROR("failed to allocate stack context list"); | ||
| goto unlock; | ||
| } | ||
|
|
||
| int ret_list = bh_list_init(context_list); | ||
| if (ret_list != BH_LIST_SUCCESS) { | ||
| LOG_ERROR("failed to initialize stack context list"); | ||
| goto free_list_and_unlock; | ||
| } | ||
|
|
||
| bool ret_map = | ||
| bh_hash_map_insert(stack_context_handler.contexts, | ||
| (void *)(uintptr_t)handle, context_list); | ||
| if (!ret_map) { | ||
| LOG_ERROR("failed to insert stack context list into map"); | ||
| goto free_list_and_unlock; | ||
| } | ||
| } | ||
|
|
||
| /* Add stack context to stack context list */ | ||
| int bh_res = bh_list_insert(context_list, context); | ||
| if (bh_res != BH_LIST_SUCCESS) { | ||
| LOG_ERROR("failed to insert stack context into list"); | ||
| goto free_list_and_unlock; | ||
| } | ||
| goto unlock; | ||
|
|
||
| free_list_and_unlock: | ||
| BH_FREE(context_list); | ||
| unlock: | ||
| stack_ctx_unlock_and_unblock_sig(); | ||
| return false; | ||
| } | ||
|
|
||
| int | ||
| os_thread_create_with_prio(korp_tid *tid, thread_start_routine_t start, | ||
| void *arg, unsigned int stack_size, int prio) | ||
|
|
@@ -89,6 +254,12 @@ os_thread_create_with_prio(korp_tid *tid, thread_start_routine_t start, | |
| return BHT_OK; | ||
| } | ||
|
|
||
| int | ||
| os_thread_signal(korp_tid tid, int sig) | ||
| { | ||
| return pthread_kill(tid, sig); | ||
| } | ||
|
|
||
| int | ||
| os_thread_create(korp_tid *tid, thread_start_routine_t start, void *arg, | ||
| unsigned int stack_size) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how about resource cleanup i mentioned in #1910 (comment) ?