dotnet · pavelsavara · Nov 7, 2023 · Nov 6, 2023 · Nov 6, 2023 · Nov 6, 2023
@@ -262,7 +262,6 @@ jobs:
       isExtraPlatformsBuild: ${{ parameters.isExtraPlatformsBuild }}
       isWasmOnlyBuild: ${{ parameters.isWasmOnlyBuild }}
       runOnlyOnWasmOnlyPipelines: true
-      shouldContinueOnError: true
 
   # Disable for now
   #- template: /eng/pipelines/coreclr/perf-wasm-jobs.yml

@@ -50,7 +50,6 @@ public AssignmentTests(ITestOutputHelper testOutput)
 
         [ConditionalTheory(nameof(RunningOnChrome))]
         [MemberData("GetTestData")]
-        [ActiveIssue("https://github.com/dotnet/runtime/issues/86496", typeof(DebuggerTests), nameof(DebuggerTests.WasmMultiThreaded))]
         public async Task InspectVariableBeforeAndAfterAssignment(string clazz, JObject checkDefault, JObject checkValue, string methodName)
         {
             await SetBreakpointInMethod("debugger-test", "DebuggerTests." + clazz, "Prepare", 2);

@@ -217,7 +217,6 @@ await EvaluateAndCheck(
         [MemberData(nameof(FalseConditions))]
         [MemberData(nameof(TrueConditions))]
         [MemberData(nameof(InvalidConditions))]
-        [ActiveIssue("https://github.com/dotnet/runtime/issues/86496", typeof(DebuggerTests), nameof(DebuggerTests.WasmMultiThreaded))]
         public async Task ConditionalBreakpoint2(string function_to_call, string method_to_stop, string condition, bool bp_stop_expected)
         {
             Result [] bps = new Result[2];
@@ -700,7 +699,6 @@ public async Task StepThroughOrNonUserCodeAttributeResumeWithBp(bool justMyCodeE
         [InlineData(true, "Debugger.stepInto", "RunStepThroughWithNonUserCode", "RunStepThroughWithNonUserCode", -1, 8, "RunStepThroughWithNonUserCode", -1, 4)]
         [InlineData(false, "Debugger.resume", "RunStepThroughWithNonUserCode", "StepThroughWithNonUserCodeUserBp", 927, 8, "RunStepThroughWithNonUserCode", -1, 4)]
         [InlineData(true, "Debugger.resume", "RunStepThroughWithNonUserCode", "RunStepThroughWithNonUserCode", -1, 8, "RunStepThroughWithNonUserCode", -1, 4)]
-        [ActiveIssue("https://github.com/dotnet/runtime/issues/86496", typeof(DebuggerTests), nameof(DebuggerTests.WasmMultiThreaded))]
         public async Task StepThroughOrNonUserCodeAttributeWithUserBp(
             bool justMyCodeEnabled, string debuggingFunction, string evalFunName,
             string functionNameCheck1, int line1, int col1,

@@ -260,7 +260,6 @@ public GetPropertiesTests(ITestOutputHelper testOutput) : base(testOutput)
         [MemberData(nameof(ClassGetPropertiesTestData), parameters: false)]
         [MemberData(nameof(StructGetPropertiesTestData), parameters: true)]
         [MemberData(nameof(StructGetPropertiesTestData), parameters: false)]
-        [ActiveIssue("https://github.com/dotnet/runtime/issues/86496", typeof(DebuggerTests), nameof(DebuggerTests.WasmMultiThreaded))]
         public async Task InspectTypeInheritedMembers(string type_name, bool? own_properties, bool? accessors_only, string[] expected_names, Dictionary<string, (JObject, bool)> all_props, bool is_async) => 
             await InspectTypeInheritedMembersInternal(type_name, own_properties, accessors_only, expected_names, all_props, is_async, AutoEvaluate.Unset);
 

@@ -1163,9 +1163,19 @@ await StepAndCheck(StepKind.Resume, "dotnet://debugger-test.dll/debugger-test.cs
             );
         }
 
-        [ConditionalFact(nameof(WasmMultiThreaded))]
-        [ActiveIssue("https://github.com/dotnet/runtime/issues/86496", typeof(DebuggerTests), nameof(DebuggerTests.WasmMultiThreaded))]
-        public async Task TestDebugUsingMultiThreadedRuntime()
+        public static TheoryData<int> CountToTen()
+        {
+            var data = new TheoryData<int>();
+            for(int i=0;i<10;i++)
+            {
+                data.Add(i);
+            }
+            return data;
+        }
+
+        [ConditionalTheory(nameof(WasmMultiThreaded))]
+        [MemberData(nameof(CountToTen))]
+        public async Task TestDebugUsingMultiThreadedRuntime(int _attempt)
         {
             var bp = await SetBreakpointInMethod("debugger-test.dll", "MultiThreadedTest", "Write", 2);
             var expression = $"{{ invoke_static_method('[debugger-test] MultiThreadedTest:Run'); }}";

@@ -1222,7 +1222,6 @@ await StepAndCheck(StepKind.Into, "dotnet://Newtonsoft.Json.dll/JArray.cs", 350,
         [ConditionalTheory(nameof(RunningOnChrome))]
         [InlineData(true)]
         [InlineData(false)]
-        [ActiveIssue("https://github.com/dotnet/runtime/issues/86496", typeof(DebuggerTests), nameof(DebuggerTests.WasmMultiThreaded))]
         public async Task SkipWasmFunctionsAccordinglyJustMyCode(bool justMyCode)
         {
             await SetJustMyCode(justMyCode);

@@ -9,6 +9,8 @@
     <OutputType>library</OutputType>
     <WasmEmitSymbolMap>true</WasmEmitSymbolMap>
     <CopyLocalLockFileAssemblies>true</CopyLocalLockFileAssemblies>
+    <!-- is 40 threads too many for debugger ? -->
+    <_WasmPThreadPoolSize Condition="'$(MonoWasmBuildVariant)' == 'multithread'">10</_WasmPThreadPoolSize>
   </PropertyGroup>
   <ItemGroup>
     <!-- keep this version to make sure it will pause in the expected line -->

@@ -7,7 +7,7 @@ import { toBase64StringImpl } from "./base64";
 import cwraps from "./cwraps";
 import { VoidPtr, CharPtr } from "./types/emscripten";
 import { mono_log_warn } from "./logging";
-import { localHeapViewU8 } from "./memory";
+import { forceThreadMemoryViewRefresh, localHeapViewU8 } from "./memory";
 import { utf8ToString } from "./strings";
 const commands_received: any = new Map<number, CommandResponse>();
 commands_received.remove = function (key: number): CommandResponse { const value = this.get(key); this.delete(key); return value; };
@@ -75,6 +75,8 @@ function mono_wasm_malloc_and_set_debug_buffer(command_parameters: string) {
 }
 
 export function mono_wasm_send_dbg_command_with_parms(id: number, command_set: number, command: number, command_parameters: string, length: number, valtype: number, newvalue: number): CommandResponseResult {
+    forceThreadMemoryViewRefresh();
+
     mono_wasm_malloc_and_set_debug_buffer(command_parameters);
     cwraps.mono_wasm_send_dbg_command_with_parms(id, command_set, command, _debugger_buffer, length, valtype, newvalue.toString());
 
@@ -85,6 +87,8 @@ export function mono_wasm_send_dbg_command_with_parms(id: number, command_set: n
 }
 
 export function mono_wasm_send_dbg_command(id: number, command_set: number, command: number, command_parameters: string): CommandResponseResult {
+    forceThreadMemoryViewRefresh();
+
     mono_wasm_malloc_and_set_debug_buffer(command_parameters);
     cwraps.mono_wasm_send_dbg_command(id, command_set, command, _debugger_buffer, command_parameters.length);
 
@@ -105,14 +109,16 @@ export function mono_wasm_get_dbg_command_info(): CommandResponseResult {
 }
 
 export function mono_wasm_debugger_resume(): void {
-    //nothing
+    forceThreadMemoryViewRefresh();
 }
 
 export function mono_wasm_detach_debugger(): void {
+    forceThreadMemoryViewRefresh();
     cwraps.mono_wasm_set_is_debugger_attached(false);
 }
 
 export function mono_wasm_change_debugger_log_level(level: number): void {
+    forceThreadMemoryViewRefresh();
     cwraps.mono_wasm_change_debugger_log_level(level);
 }
 
@@ -148,6 +154,7 @@ export function mono_wasm_wait_for_debugger(): Promise<void> {
 export function mono_wasm_debugger_attached(): void {
     if (runtimeHelpers.waitForDebugger == -1)
         runtimeHelpers.waitForDebugger = 1;
+    forceThreadMemoryViewRefresh();
     cwraps.mono_wasm_set_is_debugger_attached(true);
 }
 
@@ -160,6 +167,8 @@ export function mono_wasm_set_entrypoint_breakpoint(assembly_name: CharPtr, entr
     console.assert(true, `Adding an entrypoint breakpoint ${_assembly_name_str} at method token  ${_entrypoint_method_token}`);
     // eslint-disable-next-line no-debugger
     debugger;
+
+    forceThreadMemoryViewRefresh();
 }
 
 function _create_proxy_from_object_id(objectId: string, details: any) {
@@ -210,6 +219,8 @@ function _create_proxy_from_object_id(objectId: string, details: any) {
 }
 
 export function mono_wasm_call_function_on(request: CallRequest): CFOResponse {
+    forceThreadMemoryViewRefresh();
+
     if (request.arguments != undefined && !Array.isArray(request.arguments))
         throw new Error(`"arguments" should be an array, but was ${request.arguments}`);
 
@@ -329,6 +340,7 @@ type ValueAsJsonString = {
 }
 
 export function mono_wasm_get_details(objectId: string, args = {}): ValueAsJsonString {
+    forceThreadMemoryViewRefresh();
     return _get_cfo_res_details(`dotnet:cfo_res:${objectId}`, args);
 }
 
@@ -344,6 +356,7 @@ export function mono_wasm_release_object(objectId: string): void {
 }
 
 export function mono_wasm_debugger_log(level: number, message_ptr: CharPtr): void {
+    forceThreadMemoryViewRefresh();
     const message = utf8ToString(message_ptr);
 
     if (INTERNAL["logging"] && typeof INTERNAL.logging["debugger"] === "function") {

@@ -408,3 +408,27 @@ export function isSharedArrayBuffer(buffer: any): buffer is SharedArrayBuffer {
     // See also https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Symbol/toStringTag
     return sharedArrayBufferDefined && buffer[Symbol.toStringTag] === "SharedArrayBuffer";
 }
+
+/*
+Problem: When WebWorker is suspended in the browser, the other running threads could `grow` the linear memory in the meantime. 
+After the thread is un-suspended C code may to try to de-reference pointer which is beyond it's known view. 
+This is likely V8 bug. We don't have direct evidence, just failed debugger unit tests with MT runtime.
+*/
+export function forceThreadMemoryViewRefresh() {
+    // this condition should be eliminated by rollup on non-threading builds and it would become empty method.
+    if (!MonoWasmThreads) return;
+
+    const wasmMemory = Module.getMemory();
+
+    /*
+    Normally when wasm memory grows in v8, this size change is broadcast to other web workers via an 'interrupt', which works by setting a thread-local flag that needs to be checked. 
+    It's possible that at this point in execution the flag has not been checked yet (because this worker was suspended by the debugger in an unknown location), 
+    which means the size change has not taken effect in this worker. 
+    wasmMemory.grow's implementation in v8 checks to see whether other workers have already grown the buffer, 
+    and will update the current worker's knowledge of the buffer's size. 
+    After that we should be able to safely updateMemoryViews and get a correctly sized view. 
+    This only works because their implementation does not skip doing work even when you ask to grow by 0 pages.
+    */
+    wasmMemory.grow(0);
+    runtimeHelpers.updateMemoryViews();
+}