From 0f5b73ae67f4a1497325b4c1d6a8f1b15d4684f4 Mon Sep 17 00:00:00 2001 From: Wenyong Huang Date: Mon, 31 Oct 2022 11:48:07 +0800 Subject: [PATCH] Add mutex initializer for wasm-c-api engine operations (#1656) The host embedder may new/delete wasm-c-api engine simultaneously in multiple threads, which requires lock for the operations. Since there isn't one time called global init/destroy APIs provided by wasm-c-api, we define a global lock and initialize it with thread mutex initializer if the platform supports that, and use it to lock the operations of engine. If the platform doesn't support thread mutex initializer, we require developer to create the lock by himself to ensure the thread-safe of the engine operations. --- core/iwasm/common/wasm_c_api.c | 78 +++++++++++++------ core/iwasm/common/wasm_c_api_internal.h | 2 +- core/iwasm/include/wasm_c_api.h | 13 ++++ .../platform/android/platform_internal.h | 2 + .../platform/darwin/platform_internal.h | 2 + .../platform/esp-idf/platform_internal.h | 2 + .../platform/linux-sgx/platform_internal.h | 4 + .../shared/platform/linux/platform_internal.h | 2 + .../shared/platform/nuttx/platform_internal.h | 2 + .../platform/vxworks/platform_internal.h | 2 + .../platform/windows/platform_internal.h | 11 +++ core/shared/platform/windows/win_thread.c | 15 ++++ 12 files changed, 111 insertions(+), 24 deletions(-) diff --git a/core/iwasm/common/wasm_c_api.c b/core/iwasm/common/wasm_c_api.c index 4b1958fe2..2c0b5d5d8 100644 --- a/core/iwasm/common/wasm_c_api.c +++ b/core/iwasm/common/wasm_c_api.c @@ -327,45 +327,77 @@ wasm_engine_new_internal(mem_alloc_type_t type, const MemAllocOption *opts) /* global engine instance */ static wasm_engine_t *singleton_engine = NULL; +#if defined(OS_THREAD_MUTEX_INITIALIZER) +/** + * lock for the singleton_engine + * Note: if the platform has mutex initializer, we use a global lock to + * lock the operations of the singleton_engine, otherwise when there are + * operations happening simultaneously in multiple threads, developer + * must create the lock by himself, and use it to lock the operations + */ +static korp_mutex engine_lock = OS_THREAD_MUTEX_INITIALIZER; +#endif + +own wasm_engine_t * +wasm_engine_new_with_args(mem_alloc_type_t type, const MemAllocOption *opts) +{ +#if defined(OS_THREAD_MUTEX_INITIALIZER) + os_mutex_lock(&engine_lock); +#endif + + if (!singleton_engine) { + singleton_engine = wasm_engine_new_internal(type, opts); + } + + if (singleton_engine) { + singleton_engine->ref_count++; + } + +#if defined(OS_THREAD_MUTEX_INITIALIZER) + os_mutex_unlock(&engine_lock); +#endif + + return singleton_engine; +} own wasm_engine_t * wasm_engine_new() { - if (!singleton_engine) { - singleton_engine = - wasm_engine_new_internal(Alloc_With_System_Allocator, NULL); - } - if (singleton_engine) - singleton_engine->ref_count++; - return singleton_engine; + return wasm_engine_new_with_args(Alloc_With_System_Allocator, NULL); } own wasm_engine_t * wasm_engine_new_with_config(own wasm_config_t *config) { (void)config; - return wasm_engine_new(); + return wasm_engine_new_with_args(Alloc_With_System_Allocator, NULL); } -own wasm_engine_t * -wasm_engine_new_with_args(mem_alloc_type_t type, const MemAllocOption *opts) -{ - if (!singleton_engine) { - singleton_engine = wasm_engine_new_internal(type, opts); - } - if (singleton_engine) - singleton_engine->ref_count++; - return singleton_engine; -} - -/* BE AWARE: will RESET the singleton */ void wasm_engine_delete(wasm_engine_t *engine) { - if (engine && (--engine->ref_count == 0)) { - wasm_engine_delete_internal(engine); - singleton_engine = NULL; +#if defined(OS_THREAD_MUTEX_INITIALIZER) + os_mutex_lock(&engine_lock); +#endif + + if (!engine || !singleton_engine || engine != singleton_engine) { +#if defined(OS_THREAD_MUTEX_INITIALIZER) + os_mutex_unlock(&engine_lock); +#endif + return; } + + if (singleton_engine->ref_count > 0) { + singleton_engine->ref_count--; + if (singleton_engine->ref_count == 0) { + wasm_engine_delete_internal(engine); + singleton_engine = NULL; + } + } + +#if defined(OS_THREAD_MUTEX_INITIALIZER) + os_mutex_unlock(&engine_lock); +#endif } wasm_store_t * diff --git a/core/iwasm/common/wasm_c_api_internal.h b/core/iwasm/common/wasm_c_api_internal.h index 95bc5fac1..ac1c93313 100644 --- a/core/iwasm/common/wasm_c_api_internal.h +++ b/core/iwasm/common/wasm_c_api_internal.h @@ -27,7 +27,7 @@ WASM_DECLARE_VEC(store, *) struct wasm_engine_t { /* support one store for now */ wasm_store_vec_t *stores; - uint32_t ref_count; + uint32 ref_count; }; struct wasm_store_t { diff --git a/core/iwasm/include/wasm_c_api.h b/core/iwasm/include/wasm_c_api.h index 848323646..62113b20c 100644 --- a/core/iwasm/include/wasm_c_api.h +++ b/core/iwasm/include/wasm_c_api.h @@ -145,6 +145,19 @@ WASM_API_EXTERN own wasm_config_t* wasm_config_new(void); WASM_DECLARE_OWN(engine) +/** + * Create a new engine + * + * Note: for the engine new/delete operations, including this, + * wasm_engine_new_with_config, wasm_engine_new_with_args, and + * wasm_engine_delete, if the platform has mutex initializer, + * then they are thread-safe: we use a global lock to lock the + * operations of the engine. Otherwise they are not thread-safe: + * when there are engine new/delete operations happening + * simultaneously in multiple threads, developer must create + * the lock by himself, and add the lock when calling these + * functions. + */ WASM_API_EXTERN own wasm_engine_t* wasm_engine_new(void); WASM_API_EXTERN own wasm_engine_t* wasm_engine_new_with_config(own wasm_config_t*); diff --git a/core/shared/platform/android/platform_internal.h b/core/shared/platform/android/platform_internal.h index 3adc8726e..521fa0c55 100644 --- a/core/shared/platform/android/platform_internal.h +++ b/core/shared/platform/android/platform_internal.h @@ -57,6 +57,8 @@ typedef pthread_cond_t korp_cond; typedef pthread_t korp_thread; typedef sem_t korp_sem; +#define OS_THREAD_MUTEX_INITIALIZER PTHREAD_MUTEX_INITIALIZER + #define os_thread_local_attribute __thread #define bh_socket_t int diff --git a/core/shared/platform/darwin/platform_internal.h b/core/shared/platform/darwin/platform_internal.h index 284e376e7..3fd1c258e 100644 --- a/core/shared/platform/darwin/platform_internal.h +++ b/core/shared/platform/darwin/platform_internal.h @@ -60,6 +60,8 @@ typedef pthread_cond_t korp_cond; typedef pthread_t korp_thread; typedef sem_t korp_sem; +#define OS_THREAD_MUTEX_INITIALIZER PTHREAD_MUTEX_INITIALIZER + #define os_thread_local_attribute __thread #define bh_socket_t int diff --git a/core/shared/platform/esp-idf/platform_internal.h b/core/shared/platform/esp-idf/platform_internal.h index 7b4d35ccd..81304ea80 100644 --- a/core/shared/platform/esp-idf/platform_internal.h +++ b/core/shared/platform/esp-idf/platform_internal.h @@ -41,6 +41,8 @@ typedef pthread_cond_t korp_cond; typedef pthread_t korp_thread; typedef unsigned int korp_sem; +#define OS_THREAD_MUTEX_INITIALIZER PTHREAD_MUTEX_INITIALIZER + #define BH_APPLET_PRESERVED_STACK_SIZE (2 * BH_KB) /* Default thread priority */ diff --git a/core/shared/platform/linux-sgx/platform_internal.h b/core/shared/platform/linux-sgx/platform_internal.h index 5670052cd..966b8ffce 100644 --- a/core/shared/platform/linux-sgx/platform_internal.h +++ b/core/shared/platform/linux-sgx/platform_internal.h @@ -52,6 +52,10 @@ typedef pthread_mutex_t korp_mutex; typedef pthread_cond_t korp_cond; typedef unsigned int korp_sem; +#ifndef SGX_DISABLE_PTHREAD +#define OS_THREAD_MUTEX_INITIALIZER PTHREAD_MUTEX_INITIALIZER +#endif + typedef int (*os_print_function_t)(const char *message); void os_set_print_function(os_print_function_t pf); diff --git a/core/shared/platform/linux/platform_internal.h b/core/shared/platform/linux/platform_internal.h index 112691425..0ac63cf5e 100644 --- a/core/shared/platform/linux/platform_internal.h +++ b/core/shared/platform/linux/platform_internal.h @@ -57,6 +57,8 @@ typedef pthread_cond_t korp_cond; typedef pthread_t korp_thread; typedef sem_t korp_sem; +#define OS_THREAD_MUTEX_INITIALIZER PTHREAD_MUTEX_INITIALIZER + #define os_thread_local_attribute __thread #define bh_socket_t int diff --git a/core/shared/platform/nuttx/platform_internal.h b/core/shared/platform/nuttx/platform_internal.h index 41e208584..74fcc599d 100644 --- a/core/shared/platform/nuttx/platform_internal.h +++ b/core/shared/platform/nuttx/platform_internal.h @@ -42,6 +42,8 @@ typedef pthread_cond_t korp_cond; typedef pthread_t korp_thread; typedef sem_t korp_sem; +#define OS_THREAD_MUTEX_INITIALIZER PTHREAD_MUTEX_INITIALIZER + #define BH_APPLET_PRESERVED_STACK_SIZE (2 * BH_KB) /* Default thread priority */ diff --git a/core/shared/platform/vxworks/platform_internal.h b/core/shared/platform/vxworks/platform_internal.h index 25ea8b4ad..f72f60322 100644 --- a/core/shared/platform/vxworks/platform_internal.h +++ b/core/shared/platform/vxworks/platform_internal.h @@ -56,6 +56,8 @@ typedef pthread_cond_t korp_cond; typedef pthread_t korp_thread; typedef sem_t korp_sem; +#define OS_THREAD_MUTEX_INITIALIZER PTHREAD_MUTEX_INITIALIZER + #define os_thread_local_attribute __thread #if WASM_DISABLE_HW_BOUND_CHECK == 0 diff --git a/core/shared/platform/windows/platform_internal.h b/core/shared/platform/windows/platform_internal.h index 03f75ace7..1bce192ee 100644 --- a/core/shared/platform/windows/platform_internal.h +++ b/core/shared/platform/windows/platform_internal.h @@ -56,6 +56,17 @@ typedef void *korp_tid; typedef void *korp_mutex; typedef void *korp_sem; +/** + * Create the mutex when os_mutex_lock is called, and no need to + * CloseHandle() for the static lock's lifetime, since + * "The system closes the handle automatically when the process + * terminates. The mutex object is destroyed when its last + * handle has been closed." + * Refer to: + * https://learn.microsoft.com/en-us/windows/win32/api/synchapi/nf-synchapi-createmutexa + */ +#define OS_THREAD_MUTEX_INITIALIZER NULL + struct os_thread_wait_node; typedef struct os_thread_wait_node *os_thread_wait_list; typedef struct korp_cond { diff --git a/core/shared/platform/windows/win_thread.c b/core/shared/platform/windows/win_thread.c index 59df9cd15..4c70e9f8a 100644 --- a/core/shared/platform/windows/win_thread.c +++ b/core/shared/platform/windows/win_thread.c @@ -512,6 +512,21 @@ os_mutex_lock(korp_mutex *mutex) int ret; assert(mutex); + + if (*mutex == NULL) { /* static initializer? */ + HANDLE p = CreateMutex(NULL, FALSE, NULL); + + if (!p) { + return BHT_ERROR; + } + + if (InterlockedCompareExchangePointer((PVOID *)mutex, (PVOID)p, NULL) + != NULL) { + /* lock has been created by other threads */ + CloseHandle(p); + } + } + ret = WaitForSingleObject(*mutex, INFINITE); return ret != WAIT_FAILED ? BHT_OK : BHT_ERROR; }