Просмотр исходного кода

Grab `cluster->lock` when modifying `exec_env->module_inst` (#2685)

Fixes: https://github.com/bytecodealliance/wasm-micro-runtime/issues/2680

And when switching back to the original module_inst, propagate exception if any.

cf. https://github.com/bytecodealliance/wasm-micro-runtime/issues/2512
YAMAMOTO Takashi 2 лет назад
Родитель
Сommit
24c4d256b3

+ 12 - 8
core/iwasm/aot/aot_runtime.c

@@ -982,7 +982,8 @@ execute_post_instantiate_functions(AOTModuleInstance *module_inst,
            wasm functions, and ensure that the exec_env's module inst
            is the correct one. */
         module_inst_main = exec_env_main->module_inst;
-        exec_env->module_inst = (WASMModuleInstanceCommon *)module_inst;
+        wasm_exec_env_set_module_inst(exec_env,
+                                      (WASMModuleInstanceCommon *)module_inst);
     }
     else {
         /* Try using the existing exec_env */
@@ -1007,7 +1008,8 @@ execute_post_instantiate_functions(AOTModuleInstance *module_inst,
                module inst to ensure that the exec_env's module inst
                is the correct one. */
             module_inst_main = exec_env->module_inst;
-            exec_env->module_inst = (WASMModuleInstanceCommon *)module_inst;
+            wasm_exec_env_set_module_inst(
+                exec_env, (WASMModuleInstanceCommon *)module_inst);
         }
     }
 
@@ -1057,12 +1059,12 @@ execute_post_instantiate_functions(AOTModuleInstance *module_inst,
 fail:
     if (is_sub_inst) {
         /* Restore the parent exec_env's module inst */
-        exec_env_main->module_inst = module_inst_main;
+        wasm_exec_env_restore_module_inst(exec_env_main, module_inst_main);
     }
     else {
         if (module_inst_main)
             /* Restore the existing exec_env's module inst */
-            exec_env->module_inst = module_inst_main;
+            wasm_exec_env_restore_module_inst(exec_env, module_inst_main);
         if (exec_env_created)
             wasm_exec_env_destroy(exec_env_created);
     }
@@ -1713,7 +1715,8 @@ execute_malloc_function(AOTModuleInstance *module_inst, WASMExecEnv *exec_env,
                module inst to ensure that the exec_env's module inst
                is the correct one. */
             module_inst_old = exec_env->module_inst;
-            exec_env->module_inst = (WASMModuleInstanceCommon *)module_inst;
+            wasm_exec_env_set_module_inst(
+                exec_env, (WASMModuleInstanceCommon *)module_inst);
         }
     }
 
@@ -1724,7 +1727,7 @@ execute_malloc_function(AOTModuleInstance *module_inst, WASMExecEnv *exec_env,
 
     if (module_inst_old)
         /* Restore the existing exec_env's module inst */
-        exec_env->module_inst = module_inst_old;
+        wasm_exec_env_restore_module_inst(exec_env, module_inst_old);
 
     if (exec_env_created)
         wasm_exec_env_destroy(exec_env_created);
@@ -1780,7 +1783,8 @@ execute_free_function(AOTModuleInstance *module_inst, WASMExecEnv *exec_env,
                module inst to ensure that the exec_env's module inst
                is the correct one. */
             module_inst_old = exec_env->module_inst;
-            exec_env->module_inst = (WASMModuleInstanceCommon *)module_inst;
+            wasm_exec_env_set_module_inst(
+                exec_env, (WASMModuleInstanceCommon *)module_inst);
         }
     }
 
@@ -1788,7 +1792,7 @@ execute_free_function(AOTModuleInstance *module_inst, WASMExecEnv *exec_env,
 
     if (module_inst_old)
         /* Restore the existing exec_env's module inst */
-        exec_env->module_inst = module_inst_old;
+        wasm_exec_env_restore_module_inst(exec_env, module_inst_old);
 
     if (exec_env_created)
         wasm_exec_env_destroy(exec_env_created);

+ 45 - 0
core/iwasm/common/wasm_exec_env.c

@@ -201,7 +201,52 @@ void
 wasm_exec_env_set_module_inst(WASMExecEnv *exec_env,
                               WASMModuleInstanceCommon *const module_inst)
 {
+#if WASM_ENABLE_THREAD_MGR != 0
+    wasm_cluster_traverse_lock(exec_env);
+#endif
     exec_env->module_inst = module_inst;
+#if WASM_ENABLE_THREAD_MGR != 0
+    wasm_cluster_traverse_unlock(exec_env);
+#endif
+}
+
+void
+wasm_exec_env_restore_module_inst(
+    WASMExecEnv *exec_env, WASMModuleInstanceCommon *const module_inst_common)
+{
+    WASMModuleInstanceCommon *old_module_inst_common = exec_env->module_inst;
+    WASMModuleInstance *old_module_inst =
+        (WASMModuleInstance *)old_module_inst_common;
+    WASMModuleInstance *module_inst = (WASMModuleInstance *)module_inst_common;
+    char cur_exception[EXCEPTION_BUF_LEN];
+
+#if WASM_ENABLE_THREAD_MGR != 0
+    wasm_cluster_traverse_lock(exec_env);
+#endif
+    exec_env->module_inst = module_inst_common;
+    /*
+     * propagate an exception if any.
+     */
+    exception_lock(old_module_inst);
+    if (old_module_inst->cur_exception[0] != '\0') {
+        bh_memcpy_s(cur_exception, sizeof(cur_exception),
+                    old_module_inst->cur_exception,
+                    sizeof(old_module_inst->cur_exception));
+    }
+    else {
+        cur_exception[0] = '\0';
+    }
+    exception_unlock(old_module_inst);
+#if WASM_ENABLE_THREAD_MGR != 0
+    wasm_cluster_traverse_unlock(exec_env);
+#endif
+    if (cur_exception[0] != '\0') {
+        exception_lock(module_inst);
+        bh_memcpy_s(module_inst->cur_exception,
+                    sizeof(module_inst->cur_exception), cur_exception,
+                    sizeof(cur_exception));
+        exception_unlock(module_inst);
+    }
 }
 
 void

+ 4 - 0
core/iwasm/common/wasm_exec_env.h

@@ -291,6 +291,10 @@ void
 wasm_exec_env_set_module_inst(
     WASMExecEnv *exec_env, struct WASMModuleInstanceCommon *const module_inst);
 
+void
+wasm_exec_env_restore_module_inst(
+    WASMExecEnv *exec_env, struct WASMModuleInstanceCommon *const module_inst);
+
 void
 wasm_exec_env_set_thread_info(WASMExecEnv *exec_env);
 

+ 4 - 10
core/iwasm/interpreter/wasm_interp_classic.c

@@ -1013,7 +1013,8 @@ wasm_interp_call_func_import(WASMModuleInstance *module_inst,
     }
 
     /* - module_inst */
-    exec_env->module_inst = (WASMModuleInstanceCommon *)sub_module_inst;
+    wasm_exec_env_set_module_inst(exec_env,
+                                  (WASMModuleInstanceCommon *)sub_module_inst);
     /* - aux_stack_boundary */
     aux_stack_origin_boundary = exec_env->aux_stack_boundary.boundary;
     exec_env->aux_stack_boundary.boundary =
@@ -1035,15 +1036,8 @@ wasm_interp_call_func_import(WASMModuleInstance *module_inst,
     prev_frame->ip = ip;
     exec_env->aux_stack_boundary.boundary = aux_stack_origin_boundary;
     exec_env->aux_stack_bottom.bottom = aux_stack_origin_bottom;
-    exec_env->module_inst = (WASMModuleInstanceCommon *)module_inst;
-
-    /* transfer exception if it is thrown */
-    if (wasm_copy_exception(sub_module_inst, NULL)) {
-        bh_memcpy_s(module_inst->cur_exception,
-                    sizeof(module_inst->cur_exception),
-                    sub_module_inst->cur_exception,
-                    sizeof(sub_module_inst->cur_exception));
-    }
+    wasm_exec_env_restore_module_inst(exec_env,
+                                      (WASMModuleInstanceCommon *)module_inst);
 }
 #endif
 

+ 4 - 10
core/iwasm/interpreter/wasm_interp_fast.c

@@ -1031,7 +1031,8 @@ wasm_interp_call_func_import(WASMModuleInstance *module_inst,
     }
 
     /* - module_inst */
-    exec_env->module_inst = (WASMModuleInstanceCommon *)sub_module_inst;
+    wasm_exec_env_set_module_inst(exec_env,
+                                  (WASMModuleInstanceCommon *)sub_module_inst);
     /* - aux_stack_boundary */
     aux_stack_origin_boundary = exec_env->aux_stack_boundary.boundary;
     exec_env->aux_stack_boundary.boundary =
@@ -1053,15 +1054,8 @@ wasm_interp_call_func_import(WASMModuleInstance *module_inst,
     prev_frame->ip = ip;
     exec_env->aux_stack_boundary.boundary = aux_stack_origin_boundary;
     exec_env->aux_stack_bottom.bottom = aux_stack_origin_bottom;
-    exec_env->module_inst = (WASMModuleInstanceCommon *)module_inst;
-
-    /* transfer exception if it is thrown */
-    if (wasm_copy_exception(sub_module_inst, NULL)) {
-        bh_memcpy_s(module_inst->cur_exception,
-                    sizeof(module_inst->cur_exception),
-                    sub_module_inst->cur_exception,
-                    sizeof(sub_module_inst->cur_exception));
-    }
+    wasm_exec_env_restore_module_inst(exec_env,
+                                      (WASMModuleInstanceCommon *)module_inst);
 }
 #endif
 

+ 12 - 8
core/iwasm/interpreter/wasm_runtime.c

@@ -1060,7 +1060,8 @@ execute_post_instantiate_functions(WASMModuleInstance *module_inst,
            wasm functions, and ensure that the exec_env's module inst
            is the correct one. */
         module_inst_main = exec_env_main->module_inst;
-        exec_env->module_inst = (WASMModuleInstanceCommon *)module_inst;
+        wasm_exec_env_set_module_inst(exec_env,
+                                      (WASMModuleInstanceCommon *)module_inst);
     }
     else {
         /* Try using the existing exec_env */
@@ -1085,7 +1086,8 @@ execute_post_instantiate_functions(WASMModuleInstance *module_inst,
                module inst to ensure that the exec_env's module inst
                is the correct one. */
             module_inst_main = exec_env->module_inst;
-            exec_env->module_inst = (WASMModuleInstanceCommon *)module_inst;
+            wasm_exec_env_set_module_inst(
+                exec_env, (WASMModuleInstanceCommon *)module_inst);
         }
     }
 
@@ -1118,12 +1120,12 @@ execute_post_instantiate_functions(WASMModuleInstance *module_inst,
 fail:
     if (is_sub_inst) {
         /* Restore the parent exec_env's module inst */
-        exec_env_main->module_inst = module_inst_main;
+        wasm_exec_env_restore_module_inst(exec_env_main, module_inst_main);
     }
     else {
         if (module_inst_main)
             /* Restore the existing exec_env's module inst */
-            exec_env->module_inst = module_inst_main;
+            wasm_exec_env_restore_module_inst(exec_env, module_inst_main);
         if (exec_env_created)
             wasm_exec_env_destroy(exec_env_created);
     }
@@ -1192,7 +1194,8 @@ execute_malloc_function(WASMModuleInstance *module_inst, WASMExecEnv *exec_env,
                module inst to ensure that the exec_env's module inst
                is the correct one. */
             module_inst_old = exec_env->module_inst;
-            exec_env->module_inst = (WASMModuleInstanceCommon *)module_inst;
+            wasm_exec_env_set_module_inst(
+                exec_env, (WASMModuleInstanceCommon *)module_inst);
         }
     }
 
@@ -1203,7 +1206,7 @@ execute_malloc_function(WASMModuleInstance *module_inst, WASMExecEnv *exec_env,
 
     if (module_inst_old)
         /* Restore the existing exec_env's module inst */
-        exec_env->module_inst = module_inst_old;
+        wasm_exec_env_restore_module_inst(exec_env, module_inst_old);
 
     if (exec_env_created)
         wasm_exec_env_destroy(exec_env_created);
@@ -1259,7 +1262,8 @@ execute_free_function(WASMModuleInstance *module_inst, WASMExecEnv *exec_env,
                module inst to ensure that the exec_env's module inst
                is the correct one. */
             module_inst_old = exec_env->module_inst;
-            exec_env->module_inst = (WASMModuleInstanceCommon *)module_inst;
+            wasm_exec_env_set_module_inst(
+                exec_env, (WASMModuleInstanceCommon *)module_inst);
         }
     }
 
@@ -1267,7 +1271,7 @@ execute_free_function(WASMModuleInstance *module_inst, WASMExecEnv *exec_env,
 
     if (module_inst_old)
         /* Restore the existing exec_env's module inst */
-        exec_env->module_inst = module_inst_old;
+        wasm_exec_env_restore_module_inst(exec_env, module_inst_old);
 
     if (exec_env_created)
         wasm_exec_env_destroy(exec_env_created);

+ 16 - 0
core/iwasm/libraries/thread-mgr/thread_manager.c

@@ -1406,3 +1406,19 @@ exception_unlock(WASMModuleInstance *module_inst)
 {
     os_mutex_unlock(&_exception_lock);
 }
+
+void
+wasm_cluster_traverse_lock(WASMExecEnv *exec_env)
+{
+    WASMCluster *cluster = wasm_exec_env_get_cluster(exec_env);
+    bh_assert(cluster);
+    os_mutex_lock(&cluster->lock);
+}
+
+void
+wasm_cluster_traverse_unlock(WASMExecEnv *exec_env)
+{
+    WASMCluster *cluster = wasm_exec_env_get_cluster(exec_env);
+    bh_assert(cluster);
+    os_mutex_unlock(&cluster->lock);
+}

+ 6 - 0
core/iwasm/libraries/thread-mgr/thread_manager.h

@@ -215,6 +215,12 @@ wasm_cluster_set_debug_inst(WASMCluster *cluster, WASMDebugInstance *inst);
 
 #endif /* end of WASM_ENABLE_DEBUG_INTERP != 0 */
 
+void
+wasm_cluster_traverse_lock(WASMExecEnv *exec_env);
+
+void
+wasm_cluster_traverse_unlock(WASMExecEnv *exec_env);
+
 #ifdef __cplusplus
 }
 #endif