-
Notifications
You must be signed in to change notification settings - Fork 5.2k
[wasm][mt] Guard more places for blocking wait on JS interop threads #98508
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[wasm][mt] Guard more places for blocking wait on JS interop threads #98508
Conversation
Extend the tests as well
|
/azp run runtime-wasm |
|
Azure Pipelines successfully started running 1 pipeline(s). |
...eropServices.JavaScript.UnitTests/System/Runtime/InteropServices/JavaScript/WebWorkerTest.cs
Show resolved
Hide resolved
...Services.JavaScript.UnitTests/System/Runtime/InteropServices/JavaScript/WebWorkerTestBase.cs
Outdated
Show resolved
Hide resolved
|
Tagging subscribers to 'arch-wasm': @lewing Issue DetailsExtend the tests as well
|
|
Log |
|
With #91317 in place, we get asserts from xharness. @maraf @pavelsavara I think we will need to make xharness to not block on main thread or find a way how to do forced blocking waits in xharness. |
We have |
Yes, by default MT tests start in background https://github.com/dotnet/runtime/blob/main/eng/testing/tests.browser.targets#L90 @radekdoulik |
I see, I guess this wasn't working locally because I was setting |
|
/azp run runtime-wasm |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| await executor.Execute(async () => | ||
| { | ||
| await Task.Delay(10, cts.Token).ContinueWith(_ => | ||
| await ForceBlockingWaitAsync(async () => |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't agree that making Task.Delay forbidden operation on UI thread is a way forward.
I think we have to find a way that it never calls blocking .Wait
Here you are just hiding the problem from the unit test, rigth ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And the same for Timer, there is no theoretical reason why this needs to block.
Except maybe for creating new child thread. If that's the reason, we should add ForceBlockingWaitAsync into new Thread() code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't agree that making
Task.Delayforbidden operation on UI thread is a way forward. I think we have to find a way that it never calls blocking.WaitHere you are just hiding the problem from the unit test, rigth ?
Yes, here I am forcing it to not break the current ManagedDelay_ContinueWith test.
The Task.Delay eventually ends calling System.Threading.LowLevelMonitor.AcquireCore(), which calls to native pthread_mutex_lock - that is a blocking call.
The Timer calls LowLevelMonitor.AcquireCore as well.
Details:
info: System.PlatformNotSupportedException : Blocking wait is not supported on the JS interop threads.
info: at System.Threading.Thread.AssureBlockingPossible() in /Users/rodo/git/threads-runtime/src/libraries/System.Private.CoreLib/src/System/Threading/Thread.cs:line 683
info: at System.Threading.LowLevelMonitor.AcquireCore() in /Users/rodo/git/threads-runtime/src/libraries/System.Private.CoreLib/src/System/Threading/LowLevelMonitor.Unix.cs:line 35
info: at System.Threading.LowLevelMonitor.Acquire() in /Users/rodo/git/threads-runtime/src/libraries/System.Private.CoreLib/src/System/Threading/LowLevelMonitor.cs:line 78
info: at System.Threading.WaitSubsystem.ThreadWaitInfo.TrySignalToSatisfyWait(WaitedListNode , Boolean ) in /Users/rodo/git/threads-runtime/src/libraries/System.Private.CoreLib/src/System/Threading/WaitSubsystem.ThreadWaitInfo.Unix.cs:line 448
info: at System.Threading.WaitSubsystem.WaitableObject.SignalAutoResetEvent() in /Users/rodo/git/threads-runtime/src/libraries/System.Private.CoreLib/src/System/Threading/WaitSubsystem.WaitableObject.Unix.cs:line 709
info: at System.Threading.WaitSubsystem.WaitableObject.SignalEvent(LockHolder& ) in /Users/rodo/git/threads-runtime/src/libraries/System.Private.CoreLib/src/System/Threading/WaitSubsystem.WaitableObject.Unix.cs:line 658
info: at System.Threading.WaitSubsystem.SetEvent(WaitableObject ) in /Users/rodo/git/threads-runtime/src/libraries/System.Private.CoreLib/src/System/Threading/WaitSubsystem.Unix.cs:line 254
info: at System.Threading.WaitSubsystem.SetEvent(IntPtr ) in /Users/rodo/git/threads-runtime/src/libraries/System.Private.CoreLib/src/System/Threading/WaitSubsystem.Unix.cs:line 244
info: at System.Threading.EventWaitHandle.Set() in /Users/rodo/git/threads-runtime/src/libraries/System.Private.CoreLib/src/System/Threading/EventWaitHandle.Unix.cs:line 48
info: at System.Threading.TimerQueue.SetTimerPortable(UInt32 ) in /Users/rodo/git/threads-runtime/src/libraries/System.Private.CoreLib/src/System/Threading/TimerQueue.Portable.cs:line 66
info: at System.Threading.TimerQueue.SetTimer(UInt32 ) in /Users/rodo/git/threads-runtime/src/libraries/System.Private.CoreLib/src/System/Threading/TimerQueue.Unix.cs:line 16
info: at System.Threading.TimerQueue.EnsureTimerFiresBy(UInt32 ) in /Users/rodo/git/threads-runtime/src/libraries/System.Private.CoreLib/src/System/Threading/Timer.cs:line 138
info: at System.Threading.TimerQueue.UpdateTimer(TimerQueueTimer , UInt32 , UInt32 ) in /Users/rodo/git/threads-runtime/src/libraries/System.Private.CoreLib/src/System/Threading/Timer.cs:line 368
info: at System.Threading.TimerQueueTimer.Change(UInt32 , UInt32 ) in /Users/rodo/git/threads-runtime/src/libraries/System.Private.CoreLib/src/System/Threading/Timer.cs:line 561
info: at System.Threading.TimerQueueTimer..ctor(TimerCallback , Object , UInt32 , UInt32 , Boolean ) in /Users/rodo/git/threads-runtime/src/libraries/System.Private.CoreLib/src/System/Threading/Timer.cs:line 516
info: at System.Threading.Tasks.Task.DelayPromise..ctor(UInt32 , TimeProvider ) in /Users/rodo/git/threads-runtime/src/libraries/System.Private.CoreLib/src/System/Threading/Tasks/Task.cs:line 5704
info: at System.Threading.Tasks.Task.DelayPromiseWithCancellation..ctor(UInt32 , TimeProvider , CancellationToken ) in /Users/rodo/git/threads-runtime/src/libraries/System.Private.CoreLib/src/System/Threading/Tasks/Task.cs:line 5749
info: at System.Threading.Tasks.Task.Delay(UInt32 , TimeProvider , CancellationToken ) in /Users/rodo/git/threads-runtime/src/libraries/System.Private.CoreLib/src/System/Threading/Tasks/Task.cs:line 5668
info: at System.Threading.Tasks.Task.Delay(Int32 millisecondsDelay, CancellationToken cancellationToken) in /Users/rodo/git/threads-runtime/src/libraries/System.Private.CoreLib/src/System/Threading/Tasks/Task.cs:line 5664
info: at System.Runtime.InteropServices.JavaScript.Tests.WebWorkerTest.<>c__DisplayClass15_0.<<ManagedDelay_ContinueWith>b__0>d.MoveNext() in /Users/rodo/git/threads-runtime/src/libraries/System.Runtime.InteropServices.JavaScript/tests/System.Runtime.InteropServices.JavaScript.UnitTests/System/Runtime/InteropServices/JavaScript/WebWorkerTest.cs:line 442
info: [FAIL] System.Runtime.InteropServices.JavaScript.Tests.WebWorkerTest.ThreadingTimer(executor: Main)
info: System.PlatformNotSupportedException : Blocking wait is not supported on the JS interop threads.
info: at System.Threading.Thread.AssureBlockingPossible() in /Users/rodo/git/threads-runtime/src/libraries/System.Private.CoreLib/src/System/Threading/Thread.cs:line 683
info: at System.Threading.LowLevelMonitor.AcquireCore() in /Users/rodo/git/threads-runtime/src/libraries/System.Private.CoreLib/src/System/Threading/LowLevelMonitor.Unix.cs:line 35
info: at System.Threading.LowLevelMonitor.Acquire() in /Users/rodo/git/threads-runtime/src/libraries/System.Private.CoreLib/src/System/Threading/LowLevelMonitor.cs:line 78
info: at System.Threading.WaitSubsystem.ThreadWaitInfo.TrySignalToSatisfyWait(WaitedListNode , Boolean ) in /Users/rodo/git/threads-runtime/src/libraries/System.Private.CoreLib/src/System/Threading/WaitSubsystem.ThreadWaitInfo.Unix.cs:line 448
info: at System.Threading.WaitSubsystem.WaitableObject.SignalAutoResetEvent() in /Users/rodo/git/threads-runtime/src/libraries/System.Private.CoreLib/src/System/Threading/WaitSubsystem.WaitableObject.Unix.cs:line 709
info: at System.Threading.WaitSubsystem.WaitableObject.SignalEvent(LockHolder& ) in /Users/rodo/git/threads-runtime/src/libraries/System.Private.CoreLib/src/System/Threading/WaitSubsystem.WaitableObject.Unix.cs:line 658
info: at System.Threading.WaitSubsystem.SetEvent(WaitableObject ) in /Users/rodo/git/threads-runtime/src/libraries/System.Private.CoreLib/src/System/Threading/WaitSubsystem.Unix.cs:line 254
info: at System.Threading.WaitSubsystem.SetEvent(IntPtr ) in /Users/rodo/git/threads-runtime/src/libraries/System.Private.CoreLib/src/System/Threading/WaitSubsystem.Unix.cs:line 244
info: at System.Threading.EventWaitHandle.Set() in /Users/rodo/git/threads-runtime/src/libraries/System.Private.CoreLib/src/System/Threading/EventWaitHandle.Unix.cs:line 48
info: at System.Threading.TimerQueue.SetTimerPortable(UInt32 ) in /Users/rodo/git/threads-runtime/src/libraries/System.Private.CoreLib/src/System/Threading/TimerQueue.Portable.cs:line 66
info: at System.Threading.TimerQueue.SetTimer(UInt32 ) in /Users/rodo/git/threads-runtime/src/libraries/System.Private.CoreLib/src/System/Threading/TimerQueue.Unix.cs:line 16
info: at System.Threading.TimerQueue.EnsureTimerFiresBy(UInt32 ) in /Users/rodo/git/threads-runtime/src/libraries/System.Private.CoreLib/src/System/Threading/Timer.cs:line 138
info: at System.Threading.TimerQueue.UpdateTimer(TimerQueueTimer , UInt32 , UInt32 ) in /Users/rodo/git/threads-runtime/src/libraries/System.Private.CoreLib/src/System/Threading/Timer.cs:line 368
info: at System.Threading.TimerQueueTimer.Change(UInt32 , UInt32 ) in /Users/rodo/git/threads-runtime/src/libraries/System.Private.CoreLib/src/System/Threading/Timer.cs:line 561
info: at System.Threading.TimerQueueTimer..ctor(TimerCallback , Object , UInt32 , UInt32 , Boolean ) in /Users/rodo/git/threads-runtime/src/libraries/System.Private.CoreLib/src/System/Threading/Timer.cs:line 516
info: at System.Threading.Timer.TimerSetup(TimerCallback , Object , UInt32 , UInt32 , Boolean ) in /Users/rodo/git/threads-runtime/src/libraries/System.Private.CoreLib/src/System/Threading/Timer.cs:line 905
info: at System.Threading.Timer..ctor(TimerCallback , Object , Int32 , Int32 , Boolean ) in /Users/rodo/git/threads-runtime/src/libraries/System.Private.CoreLib/src/System/Threading/Timer.cs:line 845
info: at System.Threading.Timer..ctor(TimerCallback , Object , Int32 , Int32 ) in /Users/rodo/git/threads-runtime/src/libraries/System.Private.CoreLib/src/System/Threading/Timer.cs:line 832
info: at System.Runtime.InteropServices.JavaScript.Tests.WebWorkerTest.<>c__DisplayClass11_0.<<ThreadingTimer>b__0>d.MoveNext() in /Users/rodo/git/threads-runtime/src/libraries/System.Runtime.InteropServices.JavaScript/tests/System.Runtime.InteropServices.JavaScript.UnitTests/System/Runtime/InteropServices/JavaScript/WebWorkerTest.cs:line 371
| { | ||
| throw; | ||
| } | ||
| /* ignore the local one */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mkhamoyan Radek improved it here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, will update in my PR.
…re-blocking-wait-detection-and-tests
|
Closing in favor of #99833 |
Extend the tests as well