From 9b7aa3d454cce4ff109773512d0b42f4e29ae5d7 Mon Sep 17 00:00:00 2001 From: Radek Doulik Date: Thu, 1 Feb 2024 10:23:46 +0100 Subject: [PATCH] [wasm][mt] throw from blocking wait on JS interop threads (#97052) * [wasm][mt] throw from blocking wait on JS interop threads Also add test * Fix typo * Add JSProxyContextBase conditionally * Fix non-mt browser build * Fix tests build * Do not add JS interop project reference To not mix intree references and source project * Add new flag to Monitor This replaces the previous context base and makes it possible to disable throw for blocking calls in JS interop * Remove old file * Changes from unsaved file * Wrap another blocking wait in JS interop * Do not reference src/System.Runtime.InteropServices.JavaScript.csproj * Disable failing test with blocking Wait * Update the test condition * Add missing line after conflict resolution * Fix build * Update the active issue and don't use define --- .../src/System.Net.Http.csproj | 22 +++++++++++++------ .../src/System.Net.WebSockets.Client.csproj | 14 ++++++++---- .../ref/System.Private.CoreLib.ExtraApis.cs | 11 ++++++++++ .../ref/System.Private.CoreLib.ExtraApis.txt | 1 + ....Runtime.InteropServices.JavaScript.csproj | 16 ++++---------- .../JavaScript/Interop/JavaScriptExports.cs | 3 +++ .../JavaScript/JSSynchronizationContext.cs | 4 ++++ ...me.InteropServices.JavaScript.Tests.csproj | 8 ++++++- .../JavaScript/WebWorkerTest.cs | 19 ++++++++++++++++ .../JavaScript/WebWorkerTestHelper.cs | 16 ++++++++++++++ .../CompatibilitySuppressions.Threading.xml | 4 ++++ .../src/System/Threading/Monitor.Mono.cs | 11 ++++++++++ .../DebuggerTestSuite/SteppingTests.cs | 1 + .../tests/debugger-test/debugger-test.csproj | 4 ---- .../WebAssembly/Directory.Build.props | 4 ---- 15 files changed, 106 insertions(+), 32 deletions(-) diff --git a/src/libraries/System.Net.Http/src/System.Net.Http.csproj b/src/libraries/System.Net.Http/src/System.Net.Http.csproj index 54991bd5a4d33..109f4aba860dc 100644 --- a/src/libraries/System.Net.Http/src/System.Net.Http.csproj +++ b/src/libraries/System.Net.Http/src/System.Net.Http.csproj @@ -446,37 +446,45 @@ - - - - - - - + + + + + + + + + + + + + + + diff --git a/src/libraries/System.Net.WebSockets.Client/src/System.Net.WebSockets.Client.csproj b/src/libraries/System.Net.WebSockets.Client/src/System.Net.WebSockets.Client.csproj index 294a9bceb4f12..f017401d6aa99 100644 --- a/src/libraries/System.Net.WebSockets.Client/src/System.Net.WebSockets.Client.csproj +++ b/src/libraries/System.Net.WebSockets.Client/src/System.Net.WebSockets.Client.csproj @@ -38,7 +38,6 @@ - @@ -46,15 +45,22 @@ - - - + + + + + + + + + + diff --git a/src/libraries/System.Private.CoreLib/ref/System.Private.CoreLib.ExtraApis.cs b/src/libraries/System.Private.CoreLib/ref/System.Private.CoreLib.ExtraApis.cs index 84e99da5aa050..dedbdd6e27692 100644 --- a/src/libraries/System.Private.CoreLib/ref/System.Private.CoreLib.ExtraApis.cs +++ b/src/libraries/System.Private.CoreLib/ref/System.Private.CoreLib.ExtraApis.cs @@ -37,3 +37,14 @@ public static partial class Debug public static System.Diagnostics.DebugProvider SetProvider(System.Diagnostics.DebugProvider provider) { throw null; } } } + +#if FEATURE_WASM_THREADS +namespace System.Threading +{ + public partial class Monitor + { + [ThreadStatic] + public static bool ThrowOnBlockingWaitOnJSInteropThread; + } +} +#endif diff --git a/src/libraries/System.Private.CoreLib/ref/System.Private.CoreLib.ExtraApis.txt b/src/libraries/System.Private.CoreLib/ref/System.Private.CoreLib.ExtraApis.txt index 0babd819e25d0..b09aabbd2decc 100644 --- a/src/libraries/System.Private.CoreLib/ref/System.Private.CoreLib.ExtraApis.txt +++ b/src/libraries/System.Private.CoreLib/ref/System.Private.CoreLib.ExtraApis.txt @@ -5,3 +5,4 @@ T:System.Runtime.Serialization.DeserializationToken M:System.Runtime.Serialization.SerializationInfo.StartDeserialization T:System.Diagnostics.DebugProvider M:System.Diagnostics.Debug.SetProvider(System.Diagnostics.DebugProvider) +F:System.Threading.Monitor.ThrowOnBlockingWaitOnJSInteropThread diff --git a/src/libraries/System.Runtime.InteropServices.JavaScript/src/System.Runtime.InteropServices.JavaScript.csproj b/src/libraries/System.Runtime.InteropServices.JavaScript/src/System.Runtime.InteropServices.JavaScript.csproj index 4a7f303685ba5..6a4b0af05c45b 100644 --- a/src/libraries/System.Runtime.InteropServices.JavaScript/src/System.Runtime.InteropServices.JavaScript.csproj +++ b/src/libraries/System.Runtime.InteropServices.JavaScript/src/System.Runtime.InteropServices.JavaScript.csproj @@ -74,18 +74,10 @@ - - - - - - - - - - - - + + + + diff --git a/src/libraries/System.Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/Interop/JavaScriptExports.cs b/src/libraries/System.Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/Interop/JavaScriptExports.cs index 870b2006fdf33..aeb130a108f58 100644 --- a/src/libraries/System.Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/Interop/JavaScriptExports.cs +++ b/src/libraries/System.Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/Interop/JavaScriptExports.cs @@ -225,9 +225,12 @@ public static void CompleteTask(JSMarshalerArgument* arguments_buffer) } if (holder.CallbackReady != null) { + var threadFlag = Monitor.ThrowOnBlockingWaitOnJSInteropThread; + Monitor.ThrowOnBlockingWaitOnJSInteropThread = false; #pragma warning disable CA1416 // Validate platform compatibility holder.CallbackReady?.Wait(); #pragma warning restore CA1416 // Validate platform compatibility + Monitor.ThrowOnBlockingWaitOnJSInteropThread = threadFlag; } #endif var callback = holder.Callback!; diff --git a/src/libraries/System.Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/JSSynchronizationContext.cs b/src/libraries/System.Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/JSSynchronizationContext.cs index f7fbb7fb5a79c..e6623cce82636 100644 --- a/src/libraries/System.Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/JSSynchronizationContext.cs +++ b/src/libraries/System.Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/JSSynchronizationContext.cs @@ -49,6 +49,7 @@ public static JSSynchronizationContext InstallWebWorkerInterop(bool isMainThread var ctx = new JSSynchronizationContext(isMainThread, cancellationToken); ctx.previousSynchronizationContext = SynchronizationContext.Current; SynchronizationContext.SetSynchronizationContext(ctx); + Monitor.ThrowOnBlockingWaitOnJSInteropThread = true; var proxyContext = ctx.ProxyContext; JSProxyContext.CurrentThreadContext = proxyContext; @@ -215,7 +216,10 @@ public override void Send(SendOrPostCallback d, object? state) Environment.FailFast($"JSSynchronizationContext.Send failed, ManagedThreadId: {Environment.CurrentManagedThreadId}. {Environment.NewLine} {Environment.StackTrace}"); } + var threadFlag = Monitor.ThrowOnBlockingWaitOnJSInteropThread; + Monitor.ThrowOnBlockingWaitOnJSInteropThread = false; signal.Wait(); + Monitor.ThrowOnBlockingWaitOnJSInteropThread = threadFlag; if (_isCancellationRequested) { diff --git a/src/libraries/System.Runtime.InteropServices.JavaScript/tests/System.Runtime.InteropServices.JavaScript.UnitTests/System.Runtime.InteropServices.JavaScript.Tests.csproj b/src/libraries/System.Runtime.InteropServices.JavaScript/tests/System.Runtime.InteropServices.JavaScript.UnitTests/System.Runtime.InteropServices.JavaScript.Tests.csproj index a4e88845ec080..e49a2bb455a50 100644 --- a/src/libraries/System.Runtime.InteropServices.JavaScript/tests/System.Runtime.InteropServices.JavaScript.UnitTests/System.Runtime.InteropServices.JavaScript.Tests.csproj +++ b/src/libraries/System.Runtime.InteropServices.JavaScript/tests/System.Runtime.InteropServices.JavaScript.UnitTests/System.Runtime.InteropServices.JavaScript.Tests.csproj @@ -29,6 +29,13 @@ + + + + + + + @@ -46,6 +53,5 @@ - diff --git a/src/libraries/System.Runtime.InteropServices.JavaScript/tests/System.Runtime.InteropServices.JavaScript.UnitTests/System/Runtime/InteropServices/JavaScript/WebWorkerTest.cs b/src/libraries/System.Runtime.InteropServices.JavaScript/tests/System.Runtime.InteropServices.JavaScript.UnitTests/System/Runtime/InteropServices/JavaScript/WebWorkerTest.cs index ba7ece2913241..aa8a91e4cd7a6 100644 --- a/src/libraries/System.Runtime.InteropServices.JavaScript/tests/System.Runtime.InteropServices.JavaScript.UnitTests/System/Runtime/InteropServices/JavaScript/WebWorkerTest.cs +++ b/src/libraries/System.Runtime.InteropServices.JavaScript/tests/System.Runtime.InteropServices.JavaScript.UnitTests/System/Runtime/InteropServices/JavaScript/WebWorkerTest.cs @@ -418,6 +418,25 @@ await executor.Execute(async () => }, cts.Token); } + [Theory, MemberData(nameof(GetTargetThreads))] + public async Task WaitAssertsOnJSInteropThreads(Executor executor) + { + var cts = CreateTestCaseTimeoutSource(); + await executor.Execute(Task () => + { + Exception? exception = null; + try { + Task.Delay(10, cts.Token).Wait(); + } catch (Exception ex) { + exception = ex; + } + + executor.AssertBlockingWait(exception); + + return Task.CompletedTask; + }, cts.Token); + } + [Theory, MemberData(nameof(GetTargetThreads))] public async Task ManagedYield(Executor executor) { diff --git a/src/libraries/System.Runtime.InteropServices.JavaScript/tests/System.Runtime.InteropServices.JavaScript.UnitTests/System/Runtime/InteropServices/JavaScript/WebWorkerTestHelper.cs b/src/libraries/System.Runtime.InteropServices.JavaScript/tests/System.Runtime.InteropServices.JavaScript.UnitTests/System/Runtime/InteropServices/JavaScript/WebWorkerTestHelper.cs index 24cafa2f743a5..35cdd8ff1858b 100644 --- a/src/libraries/System.Runtime.InteropServices.JavaScript/tests/System.Runtime.InteropServices.JavaScript.UnitTests/System/Runtime/InteropServices/JavaScript/WebWorkerTestHelper.cs +++ b/src/libraries/System.Runtime.InteropServices.JavaScript/tests/System.Runtime.InteropServices.JavaScript.UnitTests/System/Runtime/InteropServices/JavaScript/WebWorkerTestHelper.cs @@ -230,6 +230,22 @@ public void AssertAwaitCapturedContext() } } + public void AssertBlockingWait(Exception? exception) + { + switch (Type) + { + case ExecutorType.Main: + case ExecutorType.JSWebWorker: + Assert.NotNull(exception); + Assert.IsType(exception); + break; + case ExecutorType.NewThread: + case ExecutorType.ThreadPool: + Assert.Null(exception); + break; + } + } + public void AssertInteropThread() { switch (Type) diff --git a/src/libraries/System.Threading/src/CompatibilitySuppressions.Threading.xml b/src/libraries/System.Threading/src/CompatibilitySuppressions.Threading.xml index 9c9a072a248df..33789a1982062 100644 --- a/src/libraries/System.Threading/src/CompatibilitySuppressions.Threading.xml +++ b/src/libraries/System.Threading/src/CompatibilitySuppressions.Threading.xml @@ -100,4 +100,8 @@ CP0014 M:System.Threading.Monitor.Wait(System.Object):[T:System.Runtime.Versioning.UnsupportedOSPlatformAttribute] + + CP0002 + F:System.Threading.Monitor.ThrowOnBlockingWaitOnJSInteropThread + diff --git a/src/mono/System.Private.CoreLib/src/System/Threading/Monitor.Mono.cs b/src/mono/System.Private.CoreLib/src/System/Threading/Monitor.Mono.cs index 07aea6c347929..78038d8769e4c 100644 --- a/src/mono/System.Private.CoreLib/src/System/Threading/Monitor.Mono.cs +++ b/src/mono/System.Private.CoreLib/src/System/Threading/Monitor.Mono.cs @@ -9,6 +9,11 @@ namespace System.Threading { public static partial class Monitor { +#if FEATURE_WASM_THREADS + [ThreadStatic] + public static bool ThrowOnBlockingWaitOnJSInteropThread; +#endif + [Intrinsic] [MethodImplAttribute(MethodImplOptions.InternalCall)] // Interpreter is missing this intrinsic public static void Enter(object obj) => Enter(obj); @@ -77,6 +82,12 @@ public static bool IsEntered(object obj) public static bool Wait(object obj, int millisecondsTimeout) { ArgumentNullException.ThrowIfNull(obj); +#if FEATURE_WASM_THREADS + if (ThrowOnBlockingWaitOnJSInteropThread) + { + throw new PlatformNotSupportedException("blocking Wait is not supported on the JS interop threads."); + } +#endif return ObjWait(millisecondsTimeout, obj); } diff --git a/src/mono/browser/debugger/DebuggerTestSuite/SteppingTests.cs b/src/mono/browser/debugger/DebuggerTestSuite/SteppingTests.cs index b97122033f96b..0074c536db81a 100644 --- a/src/mono/browser/debugger/DebuggerTestSuite/SteppingTests.cs +++ b/src/mono/browser/debugger/DebuggerTestSuite/SteppingTests.cs @@ -957,6 +957,7 @@ await EvaluateAndCheck( } [Fact] + [ActiveIssue("https://github.com/dotnet/runtime/issues/97652", typeof(DebuggerTests), nameof(DebuggerTests.WasmMultiThreaded))] public async Task StepOverWithMoreThanOneCommandInSameLineAsync() { await SetBreakpoint("dotnet://debugger-test.dll/debugger-test.cs", 710, 0); diff --git a/src/mono/browser/debugger/tests/debugger-test/debugger-test.csproj b/src/mono/browser/debugger/tests/debugger-test/debugger-test.csproj index f762d30bc5b80..be87407a47183 100644 --- a/src/mono/browser/debugger/tests/debugger-test/debugger-test.csproj +++ b/src/mono/browser/debugger/tests/debugger-test/debugger-test.csproj @@ -119,9 +119,5 @@ - - - - diff --git a/src/tests/FunctionalTests/WebAssembly/Directory.Build.props b/src/tests/FunctionalTests/WebAssembly/Directory.Build.props index 3f19205474672..e0eb44a860c46 100644 --- a/src/tests/FunctionalTests/WebAssembly/Directory.Build.props +++ b/src/tests/FunctionalTests/WebAssembly/Directory.Build.props @@ -12,10 +12,6 @@ 42 - - - -