From 9cf90be1fc89950ec51b6bf8fe29340b825b4570 Mon Sep 17 00:00:00 2001 From: Radek Doulik Date: Thu, 15 Feb 2024 18:41:19 +0100 Subject: [PATCH 1/4] [wasm][mt] Guard more places for blocking wait on JS interop threads Extend the tests as well --- .../src/System/Threading/LowLevelMonitor.Unix.cs | 10 ++++++++++ .../InteropServices/JavaScript/WebWorkerTest.cs | 3 ++- .../JavaScript/WebWorkerTestBase.cs | 16 ++++++++++++++++ 3 files changed, 28 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/LowLevelMonitor.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/LowLevelMonitor.Unix.cs index 4b3dfb3be1cc4..e548aa3e8e270 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/LowLevelMonitor.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/LowLevelMonitor.Unix.cs @@ -31,6 +31,9 @@ private void DisposeCore() private void AcquireCore() { +#if FEATURE_WASM_MANAGED_THREADS + Thread.AssureBlockingPossible(); +#endif Interop.Sys.LowLevelMonitor_Acquire(_nativeMonitor); } @@ -41,6 +44,9 @@ private void ReleaseCore() private void WaitCore() { +#if FEATURE_WASM_MANAGED_THREADS + Thread.AssureBlockingPossible(); +#endif Interop.Sys.LowLevelMonitor_Wait(_nativeMonitor); } @@ -54,6 +60,10 @@ private bool WaitCore(int timeoutMilliseconds) return true; } +#if FEATURE_WASM_MANAGED_THREADS + Thread.AssureBlockingPossible(); +#endif + return Interop.Sys.LowLevelMonitor_TimedWait(_nativeMonitor, timeoutMilliseconds); } 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 514741a6b8753..f5193ad9c8c88 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 @@ -439,7 +439,8 @@ await executor.Execute(async () => [Theory, MemberData(nameof(GetTargetThreadsAndBlockingCalls))] public async Task WaitAssertsOnJSInteropThreads(Executor executor, NamedCall method) { - using var cts = CreateTestCaseTimeoutSource(); + CancellationTokenSource? cts = null; + Thread.ForceBlockingWait((_) => cts = CreateTestCaseTimeoutSource(), null); await executor.Execute(Task () => { Exception? exception = null; diff --git a/src/libraries/System.Runtime.InteropServices.JavaScript/tests/System.Runtime.InteropServices.JavaScript.UnitTests/System/Runtime/InteropServices/JavaScript/WebWorkerTestBase.cs b/src/libraries/System.Runtime.InteropServices.JavaScript/tests/System.Runtime.InteropServices.JavaScript.UnitTests/System/Runtime/InteropServices/JavaScript/WebWorkerTestBase.cs index 87f88745377b0..192502e1c7a3a 100644 --- a/src/libraries/System.Runtime.InteropServices.JavaScript/tests/System.Runtime.InteropServices.JavaScript.UnitTests/System/Runtime/InteropServices/JavaScript/WebWorkerTestBase.cs +++ b/src/libraries/System.Runtime.InteropServices.JavaScript/tests/System.Runtime.InteropServices.JavaScript.UnitTests/System/Runtime/InteropServices/JavaScript/WebWorkerTestBase.cs @@ -165,6 +165,22 @@ public class NamedCall sem.Wait(cts.Token); } catch (OperationCanceledException) { /* ignore */ } }}, + new NamedCall { Name = "CancellationTokenSource.ctor", Call = delegate (CancellationToken ct) { + using var cts = new CancellationTokenSource(8); + }}, + new NamedCall { Name = "Mutex.WaitOne", Call = delegate (CancellationToken ct) { + using var mr = new ManualResetEventSlim(false); + var mutex = new Mutex(); + var thread = new Thread(() => { + mutex.WaitOne(); + mr.Set(); + Thread.Sleep(50); + mutex.ReleaseMutex(); + }); + thread.Start(); + Thread.ForceBlockingWait((_) => mr.Wait(), null); + mutex.WaitOne(); + }}, }; public static IEnumerable GetTargetThreadsAndBlockingCalls() From 468bea07cba9414d608dca011edb872e70d55c49 Mon Sep 17 00:00:00 2001 From: Radek Doulik Date: Thu, 15 Feb 2024 22:15:44 +0100 Subject: [PATCH 2/4] Feedback --- .../System/Runtime/InteropServices/JavaScript/WebWorkerTest.cs | 2 ++ 1 file changed, 2 insertions(+) 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 f5193ad9c8c88..cea8231b46470 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 @@ -457,6 +457,8 @@ await executor.Execute(Task () => return Task.CompletedTask; }, cts.Token); + + cts?.Dispose(); } [Theory, MemberData(nameof(GetTargetThreads))] From aca8dd5247d756b509818b6f7f13179c330c8eba Mon Sep 17 00:00:00 2001 From: Radek Doulik Date: Fri, 16 Feb 2024 11:15:00 +0100 Subject: [PATCH 3/4] Feedback --- .../JavaScript/WebWorkerTest.cs | 35 +++++++++++-------- .../JavaScript/WebWorkerTestBase.cs | 24 ++++++++----- 2 files changed, 36 insertions(+), 23 deletions(-) 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 cea8231b46470..f1d14b4c0dadc 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 @@ -440,25 +440,30 @@ await executor.Execute(async () => public async Task WaitAssertsOnJSInteropThreads(Executor executor, NamedCall method) { CancellationTokenSource? cts = null; - Thread.ForceBlockingWait((_) => cts = CreateTestCaseTimeoutSource(), null); - await executor.Execute(Task () => + try { - Exception? exception = null; - try - { - method.Call(cts.Token); - } - catch (Exception ex) + Thread.ForceBlockingWait((_) => cts = CreateTestCaseTimeoutSource(), null); + await executor.Execute(Task () => { - exception = ex; - } - - executor.AssertBlockingWait(exception); + Exception? exception = null; + try + { + method.Call(cts.Token); + } + catch (Exception ex) + { + exception = ex; + } - return Task.CompletedTask; - }, cts.Token); + executor.AssertBlockingWait(exception); - cts?.Dispose(); + return Task.CompletedTask; + }, cts.Token); + } + finally + { + cts?.Dispose(); + } } [Theory, MemberData(nameof(GetTargetThreads))] diff --git a/src/libraries/System.Runtime.InteropServices.JavaScript/tests/System.Runtime.InteropServices.JavaScript.UnitTests/System/Runtime/InteropServices/JavaScript/WebWorkerTestBase.cs b/src/libraries/System.Runtime.InteropServices.JavaScript/tests/System.Runtime.InteropServices.JavaScript.UnitTests/System/Runtime/InteropServices/JavaScript/WebWorkerTestBase.cs index 192502e1c7a3a..5c84c2766b87e 100644 --- a/src/libraries/System.Runtime.InteropServices.JavaScript/tests/System.Runtime.InteropServices.JavaScript.UnitTests/System/Runtime/InteropServices/JavaScript/WebWorkerTestBase.cs +++ b/src/libraries/System.Runtime.InteropServices.JavaScript/tests/System.Runtime.InteropServices.JavaScript.UnitTests/System/Runtime/InteropServices/JavaScript/WebWorkerTestBase.cs @@ -146,6 +146,20 @@ public class NamedCall override public string ToString() => Name; } + static void LocalCtsIgnoringCall(Action action) + { + var cts = new CancellationTokenSource(8); + try { + action(cts.Token); + } catch (OperationCanceledException exception) { + if (exception.CancellationToken != cts.Token) + { + throw; + } + /* ignore the local one */ + } + } + public static IEnumerable BlockingCalls = new List { new NamedCall { Name = "Task.Wait", Call = delegate (CancellationToken ct) { Task.Delay(10, ct).Wait(ct); }}, @@ -153,17 +167,11 @@ public class NamedCall new NamedCall { Name = "Task.WaitAny", Call = delegate (CancellationToken ct) { Task.WaitAny(Task.Delay(10, ct)); }}, new NamedCall { Name = "ManualResetEventSlim.Wait", Call = delegate (CancellationToken ct) { using var mr = new ManualResetEventSlim(false); - using var cts = new CancellationTokenSource(8); - try { - mr.Wait(cts.Token); - } catch (OperationCanceledException) { /* ignore */ } + LocalCtsIgnoringCall(mr.Wait); }}, new NamedCall { Name = "SemaphoreSlim.Wait", Call = delegate (CancellationToken ct) { using var sem = new SemaphoreSlim(2); - var cts = new CancellationTokenSource(8); - try { - sem.Wait(cts.Token); - } catch (OperationCanceledException) { /* ignore */ } + LocalCtsIgnoringCall(sem.Wait); }}, new NamedCall { Name = "CancellationTokenSource.ctor", Call = delegate (CancellationToken ct) { using var cts = new CancellationTokenSource(8); From 157b48716ae27df1e649ab9667a5113c22ef41a3 Mon Sep 17 00:00:00 2001 From: Radek Doulik Date: Fri, 16 Feb 2024 19:51:36 +0100 Subject: [PATCH 4/4] Fix existing tests --- .../JavaScript/WebWorkerTest.cs | 48 ++++++++++++++----- 1 file changed, 37 insertions(+), 11 deletions(-) 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 f1d14b4c0dadc..faa70b5e93d42 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 @@ -365,15 +365,19 @@ await executor.Execute(async () => { TaskCompletionSource tcs = new TaskCompletionSource(); - using var timer = new Timer(_ => + await ForceBlockingWaitAsync(async () => { - Assert.NotEqual(1, Environment.CurrentManagedThreadId); - Assert.True(Thread.CurrentThread.IsThreadPoolThread); - tcs.SetResult(); - hit = true; - }, null, 100, Timeout.Infinite); - await tcs.Task; + using var timer = new Timer(_ => + { + Assert.NotEqual(1, Environment.CurrentManagedThreadId); + Assert.True(Thread.CurrentThread.IsThreadPoolThread); + tcs.SetResult(); + hit = true; + }, null, 100, Timeout.Infinite); + + await tcs.Task; + }); }, cts.Token); Assert.True(hit); @@ -409,6 +413,22 @@ await executor.Execute(async () => }, cts.Token); } + static async Task ForceBlockingWaitAsync(Func action) + { + var flag = Thread.ThrowOnBlockingWaitOnJSInteropThread; + try + { + Thread.ThrowOnBlockingWaitOnJSInteropThread = false; + + await action(); + } + finally + { + Thread.ThrowOnBlockingWaitOnJSInteropThread = flag; + } + } + + [Theory, MemberData(nameof(GetTargetThreads))] public async Task ManagedDelay_ContinueWith(Executor executor) { @@ -416,10 +436,13 @@ public async Task ManagedDelay_ContinueWith(Executor executor) using var cts = CreateTestCaseTimeoutSource(); await executor.Execute(async () => { - await Task.Delay(10, cts.Token).ContinueWith(_ => + await ForceBlockingWaitAsync(async () => { - hit = true; - }, TaskContinuationOptions.ExecuteSynchronously); + await Task.Delay(10, cts.Token).ContinueWith(_ => + { + hit = true; + }, TaskContinuationOptions.ExecuteSynchronously); + }); }, cts.Token); Assert.True(hit); } @@ -430,7 +453,10 @@ public async Task ManagedDelay_ConfigureAwait_True(Executor executor) using var cts = CreateTestCaseTimeoutSource(); await executor.Execute(async () => { - await Task.Delay(10, cts.Token).ConfigureAwait(true); + await ForceBlockingWaitAsync(async () => + { + await Task.Delay(10, cts.Token).ConfigureAwait(true); + }); executor.AssertAwaitCapturedContext(); }, cts.Token);