Enable lock for Vector to protect wasm-c-api read/write/extend operations (#1010)

core/iwasm/aot/aot_runtime.c

@@ -3025,7 +3025,8 @@ aot_dump_call_stack(WASMExecEnv *exec_env)
 
     /* release previous stack frames and create new ones */
     if (!bh_vector_destroy(module_inst->frames.ptr)
-        || !bh_vector_init(module_inst->frames.ptr, n, sizeof(WASMCApiFrame))) {
+        || !bh_vector_init(module_inst->frames.ptr, n, sizeof(WASMCApiFrame),
+                           false)) {
         return;
     }
 

core/iwasm/common/wasm_c_api.c

@@ -124,7 +124,8 @@ failed:                                \
             return;                                                       \
         }                                                                 \
                                                                           \
-        if (!bh_vector_init((Vector *)out, size, sizeof(wasm_##name##_t))) { \
+        if (!bh_vector_init((Vector *)out, size, sizeof(wasm_##name##_t), \
+                            true)) {                                      \
             LOG_DEBUG("bh_vector_init failed");                           \
             goto failed;                                                  \
         }                                                                 \
@@ -166,7 +167,8 @@ failed:                                \
             return;                                                         \
         }                                                                   \
                                                                             \
-        if (!bh_vector_init((Vector *)out, size, sizeof(wasm_##name##_t *))) { \
+        if (!bh_vector_init((Vector *)out, size, sizeof(wasm_##name##_t *), \
+                            true)) {                                        \
             LOG_DEBUG("bh_vector_init failed");                             \
             goto failed;                                                    \
         }                                                                   \
@@ -191,7 +193,7 @@ failed:                                \
         }                                                                   \
                                                                             \
         if (!bh_vector_init((Vector *)out, src->size,                       \
-                            sizeof(wasm_##name##_t *))) {                      \
+                            sizeof(wasm_##name##_t *), true)) {             \
             LOG_DEBUG("bh_vector_init failed");                             \
             goto failed;                                                    \
         }                                                                   \
@@ -377,7 +379,7 @@ wasm_store_new(wasm_engine_t *engine)
              DEFAULT_VECTOR_INIT_LENGTH);
 
     if (!(store->foreigns = malloc_internal(sizeof(Vector)))
-        || !(bh_vector_init(store->foreigns, 24, sizeof(Vector *)))) {
+        || !(bh_vector_init(store->foreigns, 24, sizeof(Vector *), true))) {
         goto failed;
     }
 

core/iwasm/include/wasm_c_api.h

@@ -88,6 +88,7 @@ typedef double float64_t;
     wasm_##name##_t ptr_or_none* data; \
     size_t num_elems; \
     size_t size_of_elem; \
+    void *lock; \
   } wasm_##name##_vec_t; \
   \
   WASM_API_EXTERN void wasm_##name##_vec_new_empty(own wasm_##name##_vec_t* out); \
@@ -589,8 +590,8 @@ WASM_API_EXTERN void wasm_instance_exports(const wasm_instance_t*, own wasm_exte
 
 // Vectors
 
-#define WASM_EMPTY_VEC {0, NULL, 0, 0}
+#define WASM_EMPTY_VEC {0, NULL, 0, 0, NULL}
-#define WASM_ARRAY_VEC(array) {sizeof(array)/sizeof(*(array)), array, sizeof(array)/sizeof(*(array)), sizeof(*(array))}
+#define WASM_ARRAY_VEC(array) {sizeof(array)/sizeof(*(array)), array, sizeof(array)/sizeof(*(array)), sizeof(*(array)), NULL}
 
 
 // Value Type construction short-hands

core/iwasm/interpreter/wasm_runtime.c

@@ -2504,7 +2504,8 @@ wasm_interp_dump_call_stack(struct WASMExecEnv *exec_env)
 
     /* release previous stack frames and create new ones */
     if (!bh_vector_destroy(module_inst->frames)
-        || !bh_vector_init(module_inst->frames, n, sizeof(WASMCApiFrame))) {
+        || !bh_vector_init(module_inst->frames, n, sizeof(WASMCApiFrame),
+                           false)) {
         return;
     }
 

core/shared/utils/bh_vector.c

@@ -38,15 +38,21 @@ extend_vector(Vector *vector, size_t length)
         return false;
     }
 
+    if (vector->lock)
+        os_mutex_lock(vector->lock);
     memcpy(data, vector->data, vector->size_elem * vector->max_elems);
     BH_FREE(vector->data);
+
     vector->data = data;
     vector->max_elems = length;
+    if (vector->lock)
+        os_mutex_unlock(vector->lock);
     return true;
 }
 
 bool
-bh_vector_init(Vector *vector, size_t init_length, size_t size_elem)
+bh_vector_init(Vector *vector, size_t init_length, size_t size_elem,
+               bool use_lock)
 {
     if (!vector) {
         LOG_ERROR("Init vector failed: vector is NULL.\n");
@@ -65,6 +71,26 @@ bh_vector_init(Vector *vector, size_t init_length, size_t size_elem)
     vector->size_elem = size_elem;
     vector->max_elems = init_length;
     vector->num_elems = 0;
+    vector->lock = NULL;
+
+    if (use_lock) {
+        if (!(vector->lock = wasm_runtime_malloc(sizeof(korp_mutex)))) {
+            LOG_ERROR("Init vector failed: alloc locker failed.\n");
+            bh_vector_destroy(vector);
+            return false;
+        }
+
+        if (BHT_OK != os_mutex_init(vector->lock)) {
+            LOG_ERROR("Init vector failed: init locker failed.\n");
+
+            wasm_runtime_free(vector->lock);
+            vector->lock = NULL;
+
+            bh_vector_destroy(vector);
+            return false;
+        }
+    }
+
     return true;
 }
 
@@ -81,13 +107,17 @@ bh_vector_set(Vector *vector, uint32 index, const void *elem_buf)
         return false;
     }
 
+    if (vector->lock)
+        os_mutex_lock(vector->lock);
     memcpy(vector->data + vector->size_elem * index, elem_buf,
            vector->size_elem);
+    if (vector->lock)
+        os_mutex_unlock(vector->lock);
     return true;
 }
 
 bool
-bh_vector_get(const Vector *vector, uint32 index, void *elem_buf)
+bh_vector_get(Vector *vector, uint32 index, void *elem_buf)
 {
     if (!vector || !elem_buf) {
         LOG_ERROR("Get vector elem failed: vector or elem buf is NULL.\n");
@@ -99,8 +129,12 @@ bh_vector_get(const Vector *vector, uint32 index, void *elem_buf)
         return false;
     }
 
+    if (vector->lock)
+        os_mutex_lock(vector->lock);
     memcpy(elem_buf, vector->data + vector->size_elem * index,
            vector->size_elem);
+    if (vector->lock)
+        os_mutex_unlock(vector->lock);
     return true;
 }
 
@@ -125,6 +159,8 @@ bh_vector_insert(Vector *vector, uint32 index, const void *elem_buf)
         return false;
     }
 
+    if (vector->lock)
+        os_mutex_lock(vector->lock);
     p = vector->data + vector->size_elem * vector->num_elems;
     for (i = vector->num_elems - 1; i > index; i--) {
         memcpy(p, p - vector->size_elem, vector->size_elem);
@@ -133,6 +169,8 @@ bh_vector_insert(Vector *vector, uint32 index, const void *elem_buf)
 
     memcpy(p, elem_buf, vector->size_elem);
     vector->num_elems++;
+    if (vector->lock)
+        os_mutex_unlock(vector->lock);
     return true;
 }
 
@@ -149,9 +187,13 @@ bh_vector_append(Vector *vector, const void *elem_buf)
         return false;
     }
 
+    if (vector->lock)
+        os_mutex_lock(vector->lock);
     memcpy(vector->data + vector->size_elem * vector->num_elems, elem_buf,
            vector->size_elem);
     vector->num_elems++;
+    if (vector->lock)
+        os_mutex_unlock(vector->lock);
     return true;
 }
 
@@ -171,6 +213,8 @@ bh_vector_remove(Vector *vector, uint32 index, void *old_elem_buf)
         return false;
     }
 
+    if (vector->lock)
+        os_mutex_lock(vector->lock);
     p = vector->data + vector->size_elem * index;
 
     if (old_elem_buf) {
@@ -183,6 +227,8 @@ bh_vector_remove(Vector *vector, uint32 index, void *old_elem_buf)
     }
 
     vector->num_elems--;
+    if (vector->lock)
+        os_mutex_unlock(vector->lock);
     return true;
 }
 
@@ -202,6 +248,12 @@ bh_vector_destroy(Vector *vector)
 
     if (vector->data)
         BH_FREE(vector->data);
+
+    if (vector->lock) {
+        os_mutex_destroy(vector->lock);
+        wasm_runtime_free(vector->lock);
+    }
+
     memset(vector, 0, sizeof(Vector));
     return true;
 }

core/shared/utils/bh_vector.h

@@ -23,6 +23,7 @@ typedef struct Vector {
     size_t num_elems;
     /* size of each element */
     size_t size_elem;
+    void *lock;
 } Vector;
 
 /**
@@ -35,7 +36,8 @@ typedef struct Vector {
  * @return true if success, false otherwise
  */
 bool
-bh_vector_init(Vector *vector, size_t init_length, size_t size_elem);
+bh_vector_init(Vector *vector, size_t init_length, size_t size_elem,
+               bool use_lock);
 
 /**
  * Set element of vector
@@ -60,7 +62,7 @@ bh_vector_set(Vector *vector, uint32 index, const void *elem_buf);
  * @return true if success, false otherwise
  */
 bool
-bh_vector_get(const Vector *vector, uint32 index, void *elem_buf);
+bh_vector_get(Vector *vector, uint32 index, void *elem_buf);
 
 /**
  * Insert element of vector

doc/wasm_c_api.md

@@ -3,12 +3,30 @@
 All samples come from the commit 340fd9528cc3b26d22fe30ee1628c8c3f2b8c53b
 of [wasm-c-api](https://github.com/WebAssembly/wasm-c-api).
 
-Developer can learn these *APIs* from
+Developer can learn these _APIs_ from
 [wasm.h](https://github.com/WebAssembly/wasm-c-api/blob/master/include/wasm.h).
 
 And here are [examples](https://github.com/WebAssembly/wasm-c-api/tree/master/example) which
 are helpful.
 
+## FYI
+
+- The thread model of _wasm_c_api_ is
+
+  - An `wasm_engine_t` instance may only be created once per process
+  - Every `wasm_store_t` and its objects may only be accessed in a single thread
+
+- `wasm_engine_new`, `wasm_engine_new_with_config`, `wasm_engine_new_with_args`,
+  `wasm_engine_delete`should be called in a thread-safe environment. Such
+  behaviors are not recommended, and please make sure an appropriate calling
+  sequence if it has to be.
+
+  - call `wasm_engine_new` and `wasm_engine_delete` in different threads
+  - call `wasm_engine_new` or `wasm_engine_delete` multiple times in
+    different threads
+
+## unspported list
+
 Currently WAMR supports most of the APIs, the unsupported APIs are listed as below:
 
 - References

samples/wasm-c-api/src/memory.c

@@ -34,7 +34,7 @@ void check(bool success) {
 
 void check_call(wasm_func_t* func, int i, wasm_val_t args[], int32_t expected) {
   wasm_val_t r[] = {WASM_INIT_VAL};
-  wasm_val_vec_t args_ = {i, args, i, sizeof(wasm_val_t)};
+  wasm_val_vec_t args_ = {i, args, i, sizeof(wasm_val_t), NULL};
   wasm_val_vec_t results = WASM_ARRAY_VEC(r);
   if (wasm_func_call(func, &args_, &results) || r[0].of.i32 != expected) {
     printf("> Error on result\n");
@@ -57,7 +57,7 @@ void check_call2(wasm_func_t* func, int32_t arg1, int32_t arg2, int32_t expected
 }
 
 void check_ok(wasm_func_t* func, int i, wasm_val_t args[]) {
-  wasm_val_vec_t args_ = {i, args, i, sizeof(wasm_val_t)};
+  wasm_val_vec_t args_ = {i, args, i, sizeof(wasm_val_t), NULL};
   wasm_val_vec_t results = WASM_EMPTY_VEC;
   if (wasm_func_call(func, &args_, &results)) {
     printf("> Error on result, expected empty\n");
@@ -72,7 +72,7 @@ void check_ok2(wasm_func_t* func, int32_t arg1, int32_t arg2) {
 
 void check_trap(wasm_func_t* func, int i, wasm_val_t args[]) {
   wasm_val_t r[] = {WASM_INIT_VAL};
-  wasm_val_vec_t args_ = {i, args, i, sizeof(wasm_val_t)};
+  wasm_val_vec_t args_ = {i, args, i, sizeof(wasm_val_t), NULL};
   wasm_val_vec_t results = WASM_ARRAY_VEC(r);
   own wasm_trap_t* trap = wasm_func_call(func, &args_, &results);
   if (! trap) {