瀏覽代碼

fix duplicated debug instance id in multiple native thread issue (#847)

The debug_instance_list is not protected by mutex, and the debug instance uses
the length of this list as its id. If several threads create debug instance at the
same time, they may get duplicated/same ids. This patch adds mutex to protect
this list.
Xu Jun 4 年之前
父節點
當前提交
9b0b33e4ae
共有 1 個文件被更改,包括 55 次插入13 次删除
  1. 55 13
      core/iwasm/libraries/debug-engine/debug_engine.c

+ 55 - 13
core/iwasm/libraries/debug-engine/debug_engine.c

@@ -21,11 +21,28 @@ typedef struct WASMDebugEngine {
     int platform_port;
     int platform_port;
     int process_base_port;
     int process_base_port;
     bh_list debug_instance_list;
     bh_list debug_instance_list;
+    korp_mutex instance_list_lock;
     bool active;
     bool active;
 } WASMDebugEngine;
 } WASMDebugEngine;
 
 
 static WASMDebugEngine *g_debug_engine;
 static WASMDebugEngine *g_debug_engine;
 
 
+static uint32 current_instance_id = 1;
+
+static uint32
+allocate_instance_id()
+{
+    uint32 id;
+
+    bh_assert(g_debug_engine);
+
+    os_mutex_lock(&g_debug_engine->instance_list_lock);
+    id = current_instance_id++;
+    os_mutex_unlock(&g_debug_engine->instance_list_lock);
+
+    return id;
+}
+
 static bool
 static bool
 should_stop(WASMDebugControlThread *control_thread)
 should_stop(WASMDebugControlThread *control_thread)
 {
 {
@@ -54,7 +71,7 @@ control_thread_routine(void *arg)
 
 
     control_thread->status = RUNNING;
     control_thread->status = RUNNING;
 
 
-    debug_inst->id = g_debug_engine->debug_instance_list.len + 1;
+    debug_inst->id = allocate_instance_id();
 
 
     control_thread->debug_engine = g_debug_engine;
     control_thread->debug_engine = g_debug_engine;
     control_thread->debug_instance = debug_inst;
     control_thread->debug_instance = debug_inst;
@@ -137,8 +154,11 @@ wasm_debug_control_thread_create(WASMDebugInstance *debug_instance)
     if (!control_thread->server)
     if (!control_thread->server)
         goto fail1;
         goto fail1;
 
 
+    os_mutex_lock(&g_debug_engine->instance_list_lock);
     /* create control thread success, append debug instance to debug engine */
     /* create control thread success, append debug instance to debug engine */
     bh_list_insert(&g_debug_engine->debug_instance_list, debug_instance);
     bh_list_insert(&g_debug_engine->debug_instance_list, debug_instance);
+    os_mutex_unlock(&g_debug_engine->instance_list_lock);
+
     wasm_cluster_send_signal_all(debug_instance->cluster, WAMR_SIG_STOP);
     wasm_cluster_send_signal_all(debug_instance->cluster, WAMR_SIG_STOP);
 
 
     return control_thread;
     return control_thread;
@@ -177,6 +197,15 @@ wasm_debug_engine_create()
     }
     }
     memset(engine, 0, sizeof(WASMDebugEngine));
     memset(engine, 0, sizeof(WASMDebugEngine));
 
 
+    if (os_mutex_init(&engine->instance_list_lock) != 0) {
+        wasm_runtime_free(engine);
+        LOG_ERROR("WASM Debug Engine error: failed to init mutex");
+        return NULL;
+    }
+
+    /* reset current instance id */
+    current_instance_id = 1;
+
     /* TODO: support Wasm platform in LLDB */
     /* TODO: support Wasm platform in LLDB */
     /*
     /*
     engine->control_thread =
     engine->control_thread =
@@ -191,6 +220,16 @@ wasm_debug_engine_create()
     return engine;
     return engine;
 }
 }
 
 
+void
+wasm_debug_engine_destroy()
+{
+    if (g_debug_engine) {
+        os_mutex_destroy(&g_debug_engine->instance_list_lock);
+        wasm_runtime_free(g_debug_engine);
+        g_debug_engine = NULL;
+    }
+}
+
 bool
 bool
 wasm_debug_engine_init(char *ip_addr, int platform_port, int process_port)
 wasm_debug_engine_init(char *ip_addr, int platform_port, int process_port)
 {
 {
@@ -230,15 +269,6 @@ wasm_debug_get_engine_active(void)
     return false;
     return false;
 }
 }
 
 
-void
-wasm_debug_engine_destroy()
-{
-    if (g_debug_engine) {
-        wasm_runtime_free(g_debug_engine);
-        g_debug_engine = NULL;
-    }
-}
-
 /* A debug Instance is a debug "process" in gdb remote protocol
 /* A debug Instance is a debug "process" in gdb remote protocol
    and bound to a runtime cluster */
    and bound to a runtime cluster */
 WASMDebugInstance *
 WASMDebugInstance *
@@ -276,13 +306,19 @@ wasm_debug_instance_create(WASMCluster *cluster)
 static WASMDebugInstance *
 static WASMDebugInstance *
 wasm_cluster_get_debug_instance(WASMDebugEngine *engine, WASMCluster *cluster)
 wasm_cluster_get_debug_instance(WASMDebugEngine *engine, WASMCluster *cluster)
 {
 {
-    WASMDebugInstance *instance =
-        bh_list_first_elem(&engine->debug_instance_list);
+    WASMDebugInstance *instance;
+
+    os_mutex_lock(&g_debug_engine->instance_list_lock);
+    instance = bh_list_first_elem(&engine->debug_instance_list);
     while (instance) {
     while (instance) {
-        if (instance->cluster == cluster)
+        if (instance->cluster == cluster) {
+            os_mutex_unlock(&g_debug_engine->instance_list_lock);
             return instance;
             return instance;
+        }
         instance = bh_list_elem_next(instance);
         instance = bh_list_elem_next(instance);
     }
     }
+    os_mutex_unlock(&g_debug_engine->instance_list_lock);
+
     return instance;
     return instance;
 }
 }
 
 
@@ -315,7 +351,10 @@ wasm_debug_instance_destroy(WASMCluster *cluster)
     if (instance) {
     if (instance) {
         /* destroy control thread */
         /* destroy control thread */
         wasm_debug_control_thread_destroy(instance);
         wasm_debug_control_thread_destroy(instance);
+
+        os_mutex_lock(&g_debug_engine->instance_list_lock);
         bh_list_remove(&g_debug_engine->debug_instance_list, instance);
         bh_list_remove(&g_debug_engine->debug_instance_list, instance);
+        os_mutex_unlock(&g_debug_engine->instance_list_lock);
 
 
         /* destroy all breakpoints */
         /* destroy all breakpoints */
         wasm_debug_instance_destroy_breakpoints(instance);
         wasm_debug_instance_destroy_breakpoints(instance);
@@ -700,12 +739,15 @@ wasm_exec_env_get_instance(WASMExecEnv *exec_env)
     WASMDebugInstance *instance = NULL;
     WASMDebugInstance *instance = NULL;
     bh_assert(g_debug_engine);
     bh_assert(g_debug_engine);
 
 
+    os_mutex_lock(&g_debug_engine->instance_list_lock);
     instance = bh_list_first_elem(&g_debug_engine->debug_instance_list);
     instance = bh_list_first_elem(&g_debug_engine->debug_instance_list);
     while (instance) {
     while (instance) {
         if (instance->cluster == exec_env->cluster)
         if (instance->cluster == exec_env->cluster)
             break;
             break;
         instance = bh_list_elem_next(instance);
         instance = bh_list_elem_next(instance);
     }
     }
+
+    os_mutex_unlock(&g_debug_engine->instance_list_lock);
     return instance;
     return instance;
 }
 }