[wasm] [debugger] First version of multithreaded debugging (#74820)

* First version of multithreaded debugging.

* Revert package-lock.json

* New line at package-lock.json

* Fix not used variable.

* Fix debugger on firefox.

* Rewrite code to avoid duplicated code.

* Fix where mono_init_debugger_agent_common is called.

* Remove whitespace.

* Update src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs

Co-authored-by: Ankit Jain <radical@gmail.com>

* Update src/mono/wasm/debugger/BrowserDebugProxy/DevToolsHelper.cs

Co-authored-by: Ankit Jain <radical@gmail.com>

* Update src/mono/wasm/debugger/BrowserDebugProxy/DevToolsHelper.cs

Co-authored-by: Ankit Jain <radical@gmail.com>

* [wasm] Debugger tests: support running with multithreaded runtime

* Add runtime-wasm-dbgtests pipeline with debugger tests running on a multi-threaded runtime

* Add multi-threaded debugger tests to runtime-wasm

* fix yml

* Always run the new tests when the pipeline is invoked manually

* Pass through extra build args for wasm debugger tests

* Addressing @radical comments.

* Apply suggestions from code review

Co-authored-by: Ankit Jain <radical@gmail.com>

* addressing radical comments

* Fixing tests failures and adding a schema to run a test that will only run in a multithreaded environment.

* Adding support for run debugger-tests in a multithreaded runtime.

* Fix running debugger tests for multithreaded runtime, passing sessionId where it's necessary.

* Fix CI.

* Addressing @radical comments
Adding a test case.

* Update src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs

Co-authored-by: Ankit Jain <radical@gmail.com>

* Update src/mono/wasm/debugger/DebuggerTestSuite/MiscTests.cs

Co-authored-by: Ankit Jain <radical@gmail.com>

* Update src/mono/wasm/debugger/DebuggerTestSuite/InspectorClient.cs

Co-authored-by: Ankit Jain <radical@gmail.com>

* Dictionary with the scriptId also uses sessionId.

* Addressing @radical review.

* Apply suggestions from code review

Co-authored-by: Ankit Jain <radical@gmail.com>

* Avoiding getting this error: Cannot transition thread 0x2a15360 from STATE_BLOCKING with DO_BLOCKING.
In the transport_send we don't save the thread context, we save it before the send function.

* Addressing @radical comments.

* Using more threads in unit test.

* Apply suggestions from code review

Co-authored-by: Ankit Jain <radical@gmail.com>

* Addressing @radical comments, and trying to fix ci.

* Removing unnecessary changes.

* Export function used on mini-wasm-debugger.

* Fixing line number.

* Fix run tests on release.

* fix compilation for multithread runtime

* trying to fix multithread debugger tests on ci

* trying to fix debugger tests on ci

* disabling tests on multithreaded runtime

* Update eng/pipelines/extra-platforms/runtime-extra-platforms-wasm.yml

Co-authored-by: Ankit Jain <radical@gmail.com>

* Throwing an exception if the "what" is not the one that is being get from the nextNotificationQueue.

---------

Co-authored-by: Ankit Jain <radical@gmail.com>

Author: thaystg

eng/pipelines/common/templates/wasm-debugger-tests.yml

@@ -4,6 +4,8 @@ parameters:
   isWasmOnlyBuild: false
   browser: 'chrome'
   shouldContinueOnError: false
+  extraBuildArgs: ''
+  nameSuffix: ''
   platforms: []
 
 jobs:
@@ -30,8 +32,11 @@ jobs:
     jobParameters:
       testGroup: innerloop
       isExtraPlatforms: ${{ parameters.isExtraPlatformsBuild }}
-      nameSuffix: Mono_DebuggerTests_${{ parameters.browser }}
-      buildArgs: -s mono+libs+libs.tests -c $(_BuildConfig) /p:ArchiveTests=true /p:TestWasmDebuggerTests=true /p:TestAssemblies=false /p:BrowserHost=$(_hostedOs) /p:DebuggerHost=${{ parameters.browser }}
+      ${{ if eq(parameters.nameSuffix, '') }}:
+        nameSuffix: Mono_DebuggerTests_${{ parameters.browser }}
+      ${{ else }}:
+        nameSuffix: ${{ parameters.nameSuffix }}
+      buildArgs: -s mono+libs+libs.tests -c $(_BuildConfig) /p:ArchiveTests=true /p:TestWasmDebuggerTests=true /p:TestAssemblies=false /p:BrowserHost=$(_hostedOs) /p:DebuggerHost=${{ parameters.browser }} ${{ parameters.extraBuildArgs }}
       timeoutInMinutes: 180
       # if !alwaysRun, then:
       #   if this is runtime-wasm (isWasmOnlyBuild):

eng/pipelines/extra-platforms/runtime-extra-platforms-wasm.yml

@@ -99,7 +99,7 @@ jobs:
         - WasmTestOnBrowser
         - WasmTestOnNodeJS
 
-  # Library tests with full threading 
+  # Library tests with full threading
   - template: /eng/pipelines/common/templates/wasm-library-tests.yml
     parameters:
       platforms:
@@ -212,6 +212,17 @@ jobs:
       # ff tests are unstable currently
       shouldContinueOnError: true
 
+  - template: /eng/pipelines/common/templates/wasm-debugger-tests.yml
+    parameters:
+      platforms:
+        - Browser_wasm
+        - Browser_wasm_win
+      extraBuildArgs: /p:MonoWasmBuildVariant=multithread /p:WasmEnableThreads=true
+      nameSuffix: DebuggerTests_MultiThreaded
+      alwaysRun: ${{ parameters.isWasmOnlyBuild }}
+      isExtraPlatformsBuild: ${{ parameters.isExtraPlatformsBuild }}
+      isWasmOnlyBuild: ${{ parameters.isWasmOnlyBuild }}
+
   # Disable for now
   #- template: /eng/pipelines/coreclr/perf-wasm-jobs.yml
     #parameters:

eng/pipelines/runtime-wasm-dbgtests.yml

@@ -0,0 +1,41 @@
+trigger: none
+
+variables:
+  - template: /eng/pipelines/common/variables.yml
+
+jobs:
+
+#
+# Evaluate paths
+#
+- template: /eng/pipelines/common/evaluate-default-paths.yml
+
+# Debugger tests
+- template: /eng/pipelines/common/templates/wasm-debugger-tests.yml
+  parameters:
+    platforms:
+      - Browser_wasm
+      - Browser_wasm_win
+    isExtraPlatformsBuild: ${{ parameters.isExtraPlatformsBuild }}
+    isWasmOnlyBuild: ${{ parameters.isWasmOnlyBuild }}
+
+- template: /eng/pipelines/common/templates/wasm-debugger-tests.yml
+  parameters:
+    platforms:
+      - Browser_wasm
+      - Browser_wasm_win
+    extraBuildArgs: /p:MonoWasmBuildVariant=multithread /p:WasmEnableThreads=true
+    nameSuffix: DebuggerTests_MultiThreaded
+    isExtraPlatformsBuild: ${{ parameters.isExtraPlatformsBuild }}
+    isWasmOnlyBuild: ${{ parameters.isWasmOnlyBuild }}
+
+- template: /eng/pipelines/common/templates/wasm-debugger-tests.yml
+  parameters:
+    platforms:
+      - Browser_wasm_firefox
+    browser: firefox
+    isExtraPlatformsBuild: ${{ parameters.isExtraPlatformsBuild }}
+    isWasmOnlyBuild: ${{ parameters.isWasmOnlyBuild }}
+    alwaysRun: ${{ parameters.isWasmOnlyBuild }}
+    # ff tests are unstable currently
+    shouldContinueOnError: true

src/mono/mono/component/debugger-agent.c

@@ -323,9 +323,6 @@ typedef struct {
 /*
  * Globals
  */
-#ifdef TARGET_WASM
-static DebuggerTlsData debugger_wasm_thread;
-#endif
 static AgentConfig agent_config;
 
 /*
@@ -414,7 +411,7 @@ static gint32 suspend_count;
 /* Whenever to buffer reply messages and send them together */
 static gboolean buffer_replies;
 
-#ifndef TARGET_WASM
+
 #define GET_TLS_DATA_FROM_THREAD(thread) \
 	DebuggerTlsData *tls = NULL; \
 	mono_loader_lock(); \
@@ -424,15 +421,6 @@ static gboolean buffer_replies;
 #define GET_DEBUGGER_TLS() \
 	DebuggerTlsData *tls; \
 	tls = (DebuggerTlsData *)mono_native_tls_get_value (debugger_tls_id);
-#else
-/* the thread argument is omitted on wasm, to avoid compiler warning */
-#define GET_TLS_DATA_FROM_THREAD(...) \
-	DebuggerTlsData *tls; \
-	tls = &debugger_wasm_thread;
-#define GET_DEBUGGER_TLS() \
-	DebuggerTlsData *tls; \
-	tls = &debugger_wasm_thread;
-#endif
 
 #define GET_EXTRA_SPACE_FOR_REF_FIELDS(klass) \
 	extra_space_size = 0; \
@@ -528,6 +516,8 @@ static void process_profiler_event (EventKind event, gpointer arg);
 
 static void invalidate_frames (DebuggerTlsData *tls);
 
+static void mono_init_debugger_agent_common (MonoProfilerHandle *prof);
+
 /* Callbacks used by debugger-engine */
 static MonoContext* tls_get_restore_state (void *the_tls);
 static gboolean try_process_suspend (void *tls, MonoContext *ctx, gboolean from_breakpoint);
@@ -814,25 +804,13 @@ mono_debugger_agent_init_internal (void)
 	mono_profiler_set_domain_loaded_callback (prof, appdomain_load);
 	mono_profiler_set_domain_unloading_callback (prof, appdomain_start_unload);
 	mono_profiler_set_domain_unloaded_callback (prof, appdomain_unload);
-	mono_profiler_set_thread_started_callback (prof, thread_startup);
-	mono_profiler_set_thread_stopped_callback (prof, thread_end);
 	mono_profiler_set_assembly_loaded_callback (prof, assembly_load);
 	mono_profiler_set_assembly_unloading_callback (prof, assembly_unload);
-	mono_profiler_set_jit_done_callback (prof, jit_done);
 	mono_profiler_set_jit_failed_callback (prof, jit_failed);
 	mono_profiler_set_gc_finalizing_callback (prof, gc_finalizing);
 	mono_profiler_set_gc_finalized_callback (prof, gc_finalized);
 
-	mono_native_tls_alloc (&debugger_tls_id, NULL);
-
-	/* Needed by the hash_table_new_type () call below */
-	mono_gc_base_init ();
-
-	thread_to_tls = mono_g_hash_table_new_type_internal ((GHashFunc)mono_object_hash_internal, NULL, MONO_HASH_KEY_GC, MONO_ROOT_SOURCE_DEBUGGER, NULL, "Debugger TLS Table");
-
-	tid_to_thread = mono_g_hash_table_new_type_internal (NULL, NULL, MONO_HASH_VALUE_GC, MONO_ROOT_SOURCE_DEBUGGER, NULL, "Debugger Thread Table");
-
-	tid_to_thread_obj = mono_g_hash_table_new_type_internal (NULL, NULL, MONO_HASH_VALUE_GC, MONO_ROOT_SOURCE_DEBUGGER, NULL, "Debugger Thread Object Table");
+	mono_init_debugger_agent_common (&prof);
 
 	pending_assembly_loads = g_ptr_array_new ();
 
@@ -852,7 +830,6 @@ mono_debugger_agent_init_internal (void)
 	}
 	mono_de_set_log_level (log_level, log_file);
 
-	ids_init ();
 	objrefs_init ();
 	suspend_init ();
 
@@ -1636,6 +1613,31 @@ static GHashTable *obj_to_objref;
 /* Protected by the dbg lock */
 static MonoGHashTable *suspended_objs;
 
+static void
+mono_init_debugger_agent_common (MonoProfilerHandle *prof)
+{
+	ids_init ();
+
+	event_requests = g_ptr_array_new ();
+
+	pending_assembly_loads = g_ptr_array_new ();
+
+	mono_profiler_set_thread_started_callback (*prof, thread_startup);
+	mono_profiler_set_thread_stopped_callback (*prof, thread_end);
+	mono_profiler_set_jit_done_callback (*prof, jit_done);
+	
+	mono_native_tls_alloc (&debugger_tls_id, NULL);
+
+	/* Needed by the hash_table_new_type () call below */
+	mono_gc_base_init ();
+
+	thread_to_tls = mono_g_hash_table_new_type_internal ((GHashFunc)mono_object_hash_internal, NULL, MONO_HASH_KEY_GC, MONO_ROOT_SOURCE_DEBUGGER, NULL, "Debugger TLS Table");
+
+	tid_to_thread = mono_g_hash_table_new_type_internal (NULL, NULL, MONO_HASH_VALUE_GC, MONO_ROOT_SOURCE_DEBUGGER, NULL, "Debugger Thread Table");
+
+	tid_to_thread_obj = mono_g_hash_table_new_type_internal (NULL, NULL, MONO_HASH_VALUE_GC, MONO_ROOT_SOURCE_DEBUGGER, NULL, "Debugger Thread Object Table");
+}
+
 #ifdef TARGET_WASM
 void
 mono_init_debugger_agent_for_wasm (int log_level_parm, MonoProfilerHandle *prof)
@@ -1646,23 +1648,17 @@ mono_init_debugger_agent_for_wasm (int log_level_parm, MonoProfilerHandle *prof)
 	int ntransports = 0;
 	DebuggerTransport *transports = mono_debugger_agent_get_transports (&ntransports);
 
-	ids_init();
 	objrefs = g_hash_table_new_full (NULL, NULL, NULL, mono_debugger_free_objref);
 	obj_to_objref = g_hash_table_new (NULL, NULL);
-	pending_assembly_loads = g_ptr_array_new ();
 
 	log_level = log_level_parm;
-	event_requests = g_ptr_array_new ();
+
 	vm_start_event_sent = TRUE;
 	transport = &transports [0];
 
-	memset(&debugger_wasm_thread, 0, sizeof(DebuggerTlsData));
-	mono_native_tls_alloc (&debugger_tls_id, NULL);
-	mono_native_tls_set_value (debugger_tls_id, &debugger_wasm_thread);
-
 	agent_config.enabled = TRUE;
 
-	mono_profiler_set_jit_done_callback (*prof, jit_done);
+	mono_init_debugger_agent_common (prof);
 }
 
 void
@@ -2255,14 +2251,17 @@ save_thread_context (MonoContext *ctx)
 void
 mono_wasm_save_thread_context (void)
 {
-	debugger_wasm_thread.really_suspended = TRUE;
-	mono_thread_state_init_from_current (&debugger_wasm_thread.context);
+	DebuggerTlsData* tls = mono_wasm_get_tls ();
+	tls->really_suspended = TRUE;
+	mono_thread_state_init_from_current (&tls->context);
 }
 
 DebuggerTlsData*
 mono_wasm_get_tls (void)
 {
-	return &debugger_wasm_thread;
+	MonoThread *thread = mono_thread_current ();
+	GET_TLS_DATA_FROM_THREAD (thread);
+	return tls;
 }
 #endif
 
@@ -2855,7 +2854,11 @@ wait_for_suspend (void)
 static gboolean
 is_suspended (void)
 {
+#ifdef HOST_WASM
+	return true;
+#else
 	return count_threads_to_wait_for () == 0;
+#endif
 }
 
 static void
@@ -3762,6 +3765,7 @@ process_event (EventKind event, gpointer arg, gint32 il_offset, MonoContext *ctx
 
 #ifdef TARGET_WASM
 	PRINT_DEBUG_MSG (1, "[%p] Sent %d events %s(%d), suspend=%d.\n", (gpointer) (gsize) mono_native_thread_id_get (), nevents, event_to_string (event), ecount, suspend_policy);
+	mono_wasm_save_thread_context();
 #endif
 
 	send_success = send_packet (CMD_SET_EVENT, CMD_COMPOSITE, &buf);
@@ -3910,8 +3914,9 @@ thread_startup (MonoProfiler *prof, uintptr_t tid)
 	/*
 	 * suspend_vm () could have missed this thread, so wait for a resume.
 	 */
-
+#ifndef HOST_WASM
 	suspend_current_func ();
+#endif
 }
 
 static void
@@ -9313,7 +9318,7 @@ thread_commands (int command, guint8 *p, guint8 *end, Buffer *buf)
 
 		// Wait for suspending if it already started
 		// FIXME: Races with suspend_count
-#ifndef HOST_WASI	
+#if !defined(HOST_WASI) && !defined(HOST_WASM)
 		while (!is_suspended ()) {
 			if (suspend_count)
 				wait_for_suspend ();
@@ -9498,9 +9503,7 @@ frame_commands (int command, guint8 *p, guint8 *end, Buffer *buf)
 	int objid;
 	ErrorCode err;
 	MonoThread *thread_obj;
-#ifndef TARGET_WASM
 	MonoInternalThread *thread;
-#endif
 	int pos, i, len, frame_idx;
 	StackFrame *frame;
 	MonoDebugMethodJitInfo *jit;
@@ -9514,16 +9517,10 @@ frame_commands (int command, guint8 *p, guint8 *end, Buffer *buf)
 	if (err != ERR_NONE)
 		return err;
 
-#ifndef TARGET_WASM
 	thread = THREAD_TO_INTERNAL (thread_obj);
-#endif
 	id = decode_id (p, &p, end);
 
-#ifndef TARGET_WASM
 	GET_TLS_DATA_FROM_THREAD (thread);
-#else
-	GET_TLS_DATA_FROM_THREAD ();
-#endif
 	g_assert (tls);
 
 	for (i = 0; i < tls->frame_count; ++i) {

src/mono/mono/component/mini-wasm-debugger.c

@@ -41,7 +41,7 @@ EMSCRIPTEN_KEEPALIVE gboolean mono_wasm_send_dbg_command_with_parms (int id, Mdb
 
 
 //JS functions imported that we use
-extern void mono_wasm_fire_debugger_agent_message (void);
+extern void mono_wasm_fire_debugger_agent_message_with_data (const char *data, int len);
 extern void mono_wasm_asm_loaded (const char *asm_name, const char *assembly_data, guint32 assembly_len, const char *pdb_data, guint32 pdb_len);
 
 G_END_DECLS
@@ -382,7 +382,7 @@ mono_wasm_send_dbg_command_with_parms (int id, MdbgProtCommandSet command_set, i
 		goto done;
 	}
 	MdbgProtBuffer bufWithParms;
-	buffer_init (&bufWithParms, 128);
+	m_dbgprot_buffer_init (&bufWithParms, 128);
 	m_dbgprot_buffer_add_data (&bufWithParms, data, size);
 	if (!write_value_to_buffer(&bufWithParms, valtype, newvalue)) {
 		mono_wasm_add_dbg_command_received(0, id, 0, 0);
@@ -410,11 +410,11 @@ mono_wasm_send_dbg_command (int id, MdbgProtCommandSet command_set, int command,
 	}
 	ss_calculate_framecount (NULL, NULL, TRUE, NULL, NULL);
 	MdbgProtBuffer buf;
-	buffer_init (&buf, 128);
 	gboolean no_reply;
 	MdbgProtErrorCode error = 0;
 	if (command_set == MDBGPROT_CMD_SET_VM && command == MDBGPROT_CMD_VM_INVOKE_METHOD )
 	{
+		m_dbgprot_buffer_init (&buf, 128);
 		DebuggerTlsData* tls = mono_wasm_get_tls ();
 		InvokeData invoke_data;
 		memset (&invoke_data, 0, sizeof (InvokeData));
@@ -426,6 +426,7 @@ mono_wasm_send_dbg_command (int id, MdbgProtCommandSet command_set, int command,
 		char* assembly_name = m_dbgprot_decode_string (data, &data, data + size);
 		if (assembly_name == NULL)
 		{
+			m_dbgprot_buffer_init (&buf, 128);
 			m_dbgprot_buffer_add_int (&buf, 0);
 			m_dbgprot_buffer_add_int (&buf, 0);
 		}
@@ -435,16 +436,20 @@ mono_wasm_send_dbg_command (int id, MdbgProtCommandSet command_set, int command,
 			int symfile_size = 0;
 			const unsigned char* assembly_bytes = mono_wasm_get_assembly_bytes (assembly_name, &assembly_size);
 			const unsigned char* pdb_bytes = mono_get_symfile_bytes_from_bundle (assembly_name, &symfile_size);
+			m_dbgprot_buffer_init (&buf, assembly_size + symfile_size);
 			m_dbgprot_buffer_add_byte_array (&buf, (uint8_t *) assembly_bytes, assembly_size);
 			m_dbgprot_buffer_add_byte_array (&buf, (uint8_t *) pdb_bytes, symfile_size);
 		}
 	}
 	else
+	{
+		m_dbgprot_buffer_init (&buf, 128);
 		error = mono_process_dbg_packet (id, command_set, command, &no_reply, data, data + size, &buf);
+	}
 
 	mono_wasm_add_dbg_command_received (error == MDBGPROT_ERR_NONE, id, buf.buf, buf.p-buf.buf);
 
-	buffer_free (&buf);
+	m_dbgprot_buffer_free (&buf);
 	result = TRUE;
 done:
 	MONO_EXIT_GC_UNSAFE;
@@ -454,9 +459,7 @@ mono_wasm_send_dbg_command (int id, MdbgProtCommandSet command_set, int command,
 static gboolean
 receive_debugger_agent_message (void *data, int len)
 {
-	mono_wasm_add_dbg_command_received(1, 0, data, len);
-	mono_wasm_save_thread_context();
-	mono_wasm_fire_debugger_agent_message ();
+	mono_wasm_fire_debugger_agent_message_with_data ((const char*)data, len);
 	return FALSE;
 }