From e23acfab3628616d63a2c879569b09e5c1a9dfb4 Mon Sep 17 00:00:00 2001 From: Wenyong Huang Date: Thu, 11 Aug 2022 16:57:32 +0800 Subject: [PATCH] Fix windows thread data issue and enhance windows os_mmap (#1372) Thread data should not be destroyed when thread exits, or other thread may not be able to join it. This PR saves the thread data into thread data list when thread exits, sets thread status and stores the return value, so that other thread can join it. Also set MEM_TOP_DOWN flag for Windows VirtualAlloc to yield LLVM JIT relocation error. And set opt/size level to 3 for LLVM JIT for future use, currently the flags are not used by LLVM JIT. --- core/iwasm/aot/aot_loader.c | 2 + core/shared/platform/windows/win_memmap.c | 11 +++++ core/shared/platform/windows/win_thread.c | 57 ++++++++++++++++++++--- 3 files changed, 63 insertions(+), 7 deletions(-) diff --git a/core/iwasm/aot/aot_loader.c b/core/iwasm/aot/aot_loader.c index c185150c..f938012a 100644 --- a/core/iwasm/aot/aot_loader.c +++ b/core/iwasm/aot/aot_loader.c @@ -3136,6 +3136,8 @@ aot_convert_wasm_module(WASMModule *wasm_module, char *error_buf, } option.is_jit_mode = true; + option.opt_level = 3; + option.size_level = 3; #if WASM_ENABLE_BULK_MEMORY != 0 option.enable_bulk_memory = true; #endif diff --git a/core/shared/platform/windows/win_memmap.c b/core/shared/platform/windows/win_memmap.c index 7afe7396..c4a6b075 100644 --- a/core/shared/platform/windows/win_memmap.c +++ b/core/shared/platform/windows/win_memmap.c @@ -43,6 +43,17 @@ os_mmap(void *hint, size_t size, int prot, int flags) /* integer overflow */ return NULL; +#if WASM_ENABLE_JIT != 0 + /** + * Allocate memory at the highest possible address if the + * request size is large, or LLVM JIT might report error: + * IMAGE_REL_AMD64_ADDR32NB relocation requires an ordered + * section layout. + */ + if (request_size > 10 * BH_MB) + alloc_type |= MEM_TOP_DOWN; +#endif + protect = access_to_win32_flags(prot); if (protect != PAGE_NOACCESS) { alloc_type |= MEM_COMMIT; diff --git a/core/shared/platform/windows/win_thread.c b/core/shared/platform/windows/win_thread.c index d5a32b53..59df9cd1 100644 --- a/core/shared/platform/windows/win_thread.c +++ b/core/shared/platform/windows/win_thread.c @@ -37,6 +37,10 @@ typedef struct os_thread_data { korp_mutex wait_lock; /* Waiting list of other threads who are joining this thread */ os_thread_wait_list thread_wait_list; + /* Whether the thread has exited */ + bool thread_exited; + /* Thread return value */ + void *thread_retval; } os_thread_data; static bool is_thread_sys_inited = false; @@ -44,6 +48,9 @@ static bool is_thread_sys_inited = false; /* Thread data of supervisor thread */ static os_thread_data supervisor_thread_data; +/* Thread data list lock */ +static korp_mutex thread_data_list_lock; + /* Thread data key */ static DWORD thread_data_key; @@ -90,7 +97,10 @@ os_thread_sys_init() if (!TlsSetValue(thread_data_key, &supervisor_thread_data)) goto fail4; - if ((module = GetModuleHandle((LPSTR) "kernel32"))) { + if (os_mutex_init(&thread_data_list_lock) != BHT_OK) + goto fail5; + + if ((module = GetModuleHandle((LPCTSTR) "kernel32"))) { *(void **)&GetCurrentThreadStackLimits_Kernel32 = GetProcAddress(module, "GetCurrentThreadStackLimits"); } @@ -98,6 +108,8 @@ os_thread_sys_init() is_thread_sys_inited = true; return BHT_OK; +fail5: + TlsSetValue(thread_data_key, NULL); fail4: os_cond_destroy(&supervisor_thread_data.wait_cond); fail3: @@ -113,6 +125,22 @@ void os_thread_sys_destroy() { if (is_thread_sys_inited) { + os_thread_data *thread_data, *thread_data_next; + + thread_data = supervisor_thread_data.next; + while (thread_data) { + thread_data_next = thread_data->next; + + /* Destroy resources of thread data */ + os_cond_destroy(&thread_data->wait_cond); + os_sem_destroy(&thread_data->wait_node.sem); + os_mutex_destroy(&thread_data->wait_lock); + BH_FREE(thread_data); + + thread_data = thread_data_next; + } + + os_mutex_destroy(&thread_data_list_lock); os_cond_destroy(&supervisor_thread_data.wait_cond); os_mutex_destroy(&supervisor_thread_data.wait_lock); os_sem_destroy(&supervisor_thread_data.wait_node.sem); @@ -148,13 +176,10 @@ os_thread_cleanup(void *retval) } thread_data->thread_wait_list = NULL; } + /* Set thread status and thread return value */ + thread_data->thread_exited = true; + thread_data->thread_retval = retval; os_mutex_unlock(&thread_data->wait_lock); - - /* Destroy resources */ - os_cond_destroy(&thread_data->wait_cond); - os_sem_destroy(&thread_data->wait_node.sem); - os_mutex_destroy(&thread_data->wait_lock); - BH_FREE(thread_data); } static unsigned __stdcall os_thread_wrapper(void *arg) @@ -224,6 +249,13 @@ os_thread_create_with_prio(korp_tid *p_tid, thread_start_routine_t start, os_mutex_unlock(&parent->wait_lock); goto fail4; } + + /* Add thread data into thread data list */ + os_mutex_lock(&thread_data_list_lock); + thread_data->next = supervisor_thread_data.next; + supervisor_thread_data.next = thread_data; + os_mutex_unlock(&thread_data_list_lock); + /* Wait for the thread routine to set thread_data's tid and add thread_data to thread data list */ os_cond_wait(&parent->wait_cond, &parent->wait_lock); @@ -271,6 +303,16 @@ os_thread_join(korp_tid thread, void **p_retval) bh_assert(thread_data); os_mutex_lock(&thread_data->wait_lock); + + if (thread_data->thread_exited) { + /* Thread has exited */ + if (p_retval) + *p_retval = thread_data->thread_retval; + os_mutex_unlock(&thread_data->wait_lock); + return BHT_OK; + } + + /* Thread is running */ if (!thread_data->thread_wait_list) thread_data->thread_wait_list = &curr_thread_data->wait_node; else { @@ -280,6 +322,7 @@ os_thread_join(korp_tid thread, void **p_retval) p = p->next; p->next = &curr_thread_data->wait_node; } + os_mutex_unlock(&thread_data->wait_lock); /* Wait the sem */