[wasm][mt] throw from blocking wait on JS interop threads #97052

radekdoulik · 2024-01-16T22:25:21Z

Also add test

Contributes to #76958

Also add test

dotnet-issue-labeler · 2024-01-16T22:25:29Z

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

src/libraries/System.Private.CoreLib/ref/System.Private.CoreLib.ExtraApis.cs

radekdoulik · 2024-01-18T13:13:41Z

The current approach doesn't solve the blocking wait in JS interop, which is then throwing the exception for us too.

[20:37:25] fail: MONO_WASM:    at System.Threading.Monitor.Wait(Object obj, Int32 millisecondsTimeout)
                    at System.Threading.ManualResetEventSlim.Wait(Int32 millisecondsTimeout, CancellationToken cancellationToken)
                    at System.Threading.ManualResetEventSlim.Wait()
                    at System.Runtime.InteropServices.JavaScript.JavaScriptExports.CompleteTask(JSMarshalerArgument* arguments_buffer)
                 Error: blocking Wait is not supported on the JS interop thread.
                     at wr (http://127.0.0.1:42115/_framework/dotnet.runtime.js:3:35092)
                     at Qo (http://127.0.0.1:42115/_framework/dotnet.runtime.js:3:73035)
                     at p.javaScriptExports.complete_task (http://127.0.0.1:42115/_framework/dotnet.runtime.js:3:248198)
                     at http://127.0.0.1:42115/_framework/dotnet.runtime.js:3:80721

We discussed it in chat and agreed on the introduction of a new flag to signal that the wait comes from JS interop code. The flag can be added to the Monitor class and thus simplify the thread JS interop thread detection.

…row-from-blocking-wait

To not mix intree references and source project

This replaces the previous context base and makes it possible to disable throw for blocking calls in JS interop

...s/System.Private.CoreLib/src/System/Runtime/InteropServices/JavaScript/JSProxyContextBase.cs

...rvices.JavaScript/src/System/Runtime/InteropServices/JavaScript/Interop/JavaScriptExports.cs

…row-from-blocking-wait

radekdoulik · 2024-01-19T17:04:58Z

New fail to look into:

[16:54:41] info: [2024-01-19T16:54:41.880Z] [FAIL] System.Runtime.InteropServices.JavaScript.Tests.WebWorkerTest.JSSynchronizationContext_Send_Post_Items_Cancellation
[16:54:41] info: Assert.Throws() Failure: Exception type was not an exact match
[16:54:41] info: Expected: typeof(System.OperationCanceledException)
[16:54:41] info: Actual:   typeof(System.ObjectDisposedException)
[16:54:41] info: ---- System.ObjectDisposedException : Cannot access a disposed object.
[16:54:41] info: Object name: 'System.Runtime.InteropServices.JavaScript.JSSynchronizationContext'.
[16:54:41] info:    at System.Runtime.InteropServices.JavaScript.Tests.WebWorkerTest.JSSynchronizationContext_Send_Post_Items_Cancellation()
[16:54:41] info: --- End of stack trace from previous location ---
[16:54:41] info: ----- Inner Stack Trace -----
[16:54:41] info:    at System.Runtime.InteropServices.JavaScript.JSSynchronizationContext.Send(SendOrPostCallback , Object )
[16:54:41] info:    at System.Runtime.InteropServices.JavaScript.Tests.WebWorkerTest.<>c__DisplayClass6_0.<JSSynchronizationContext_Send_Post_Items_Cancellation>b__1()
[16:54:41] info:    at System.Threading.Tasks.Task`1[[System.Threading.Tasks.Task, System.Private.CoreLib, Version=9.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]].InnerInvoke()
[16:54:41] info:    at System.Threading.Tasks.Task.<>c.<.cctor>b__281_0(Object obj)
[16:54:41] info:    at System.Threading.ExecutionContext.RunFromThreadPoolDispatchLoop(Thread , ExecutionContext , ContextCallback , Object )
[16:54:41] info: --- End of stack trace from previous location ---
[16:54:41] info:    at System.Threading.ExecutionContext.RunFromThreadPoolDispatchLoop(Thread , ExecutionContext , ContextCallback , Object )
[16:54:41] info:    at System.Threading.Tasks.Task.ExecuteWithThreadLocal(Task& , Thread )
[16:54:41] info: --- End of stack trace from previous location ---

pavelsavara · 2024-01-19T17:49:33Z

New fail to look into:

It's unrelated to your PR, please add it to #96628

azure-pipelines · 2024-01-30T12:40:55Z

Azure Pipelines successfully started running 1 pipeline(s).

radekdoulik · 2024-01-30T16:19:04Z

/azp run runtime-wasm

azure-pipelines · 2024-01-30T16:19:25Z

Azure Pipelines successfully started running 1 pipeline(s).

pavelsavara · 2024-01-31T08:36:42Z

Log

[16:55:56] fail: [FAIL] System.Runtime.InteropServices.JavaScript.Tests.WebWorkerTest.JSSynchronizationContext_Send_Post_Items_Cancellation
[16:55:56] info: Assert.ThrowsAny() Failure: Exception type was not compatible
[16:55:56] info: Expected: typeof(System.OperationCanceledException)
[16:55:56] info: Actual:   typeof(System.ObjectDisposedException)

This is unrelated.

pavelsavara · 2024-01-31T08:37:26Z

🎉

pavelsavara · 2024-01-31T09:46:01Z

Quick list of APIs, for which we need to validate that they internally use something we already addressed so far.
Or we need to address them too.

Task - WaitAll, WaitAny
WaitSubsystem - Wait
WaitableObject - .Wait, SatisfyWaitForAll, Wait_Locked
ThreadWaitInfo - .Wait, Sleep,
LowLevelLock - WaitAndAcquire
LowLevelMonitor - Wait, Acquire
LowLevelSpinWaiter - Wait
LowLevelLifoSemaphore - Wait
FileSystemWatcher - WaitForChanged
GC - WaitForFullGCApproach, WaitForFullGCComplete, WaitForPendingFinalizers
CancellationTokenSource - WaitForCallbackToComplete
ManualResetEventSlim - Wait
SemaphoreSlim - Wait
SynchronizationContext - Wait
WaitHandle - WaitOne, WaitAll
ManualResetEventWithAwaiterSupport - Wait
CountdownEvent - Wait
Interop.Sys.LowLevelMonitor_TimedWait & SystemNative_LowLevelMonitor_Wait

I'm sure I missed some places.

radekdoulik · 2024-02-01T07:21:11Z

Lets get this in and I can extend the tests and API coverage in follow up PR.

pavelsavara · 2024-02-01T08:43:22Z

src/mono/System.Private.CoreLib/src/System/Threading/Monitor.Mono.cs

@@ -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


Let's make helper method for this and localize the message.
The method should be "public" but suppressed same like ThrowOnBlockingWaitOnJSInteropThread.
So that we could use it in other places in the runtime.

pavelsavara · 2024-02-01T11:06:27Z

...ervices.JavaScript/src/System/Runtime/InteropServices/JavaScript/JSSynchronizationContext.cs

                signal.Wait();
+                Monitor.ThrowOnBlockingWaitOnJSInteropThread = threadFlag;


should this be in try-finally ?
could .Wait ever throw ?
@radekdoulik

I'll will change that on my PR #97832

pavelsavara · 2024-02-01T18:52:52Z

This is unrelated.

It was actually related but hiding behind another problem.
This is now throwing PNSE (as it should), but the test was not ready for it.

runtime/src/libraries/System.Runtime.InteropServices.JavaScript/tests/System.Runtime.InteropServices.JavaScript.UnitTests/System/Runtime/InteropServices/JavaScript/WebWorkerTest.cs

Line 88 in 5cee418

blocker.Wait();

I'm fixing it in #97832

The project files have now diferent dependencies

* Update solution files after changes in #97052 The project files have now diferent dependencies * Update System.Runtime.InteropServices.JavaScript.sln * Update System.Net.Http.sln * Update System.Net.WebSockets.Client.sln --------- Co-authored-by: Viktor Hofer <viktor.hofer@microsoft.com>

[wasm][mt] throw from blocking wait on JS interop threads

b049c6a

Also add test

dotnet-issue-labeler bot added area-System.Runtime.InteropServices.JavaScript new-api-needs-documentation labels Jan 16, 2024

ghost assigned radekdoulik Jan 16, 2024

radekdoulik added 3 commits January 17, 2024 17:05

Fix typo

150a177

Add JSProxyContextBase conditionally

a88e493

Fix non-mt browser build

61b797c

build-analysis bot mentioned this pull request Jan 17, 2024

Tracking issue for CI build timeouts #76454

Closed

radekdoulik added 2 commits January 17, 2024 20:46

Fix tests build

1ed5faa

Merge branch 'main' into pr-wasm-mt-throw-from-blocking-wait

9d24021

pavelsavara reviewed Jan 18, 2024

View reviewed changes

src/libraries/System.Private.CoreLib/ref/System.Private.CoreLib.ExtraApis.cs Outdated Show resolved Hide resolved

radekdoulik added 4 commits January 19, 2024 15:15

Merge remote-tracking branch 'remotes/origin/main' into pr-wasm-mt-th…

70ad3a6

…row-from-blocking-wait

Do not add JS interop project reference

dbe7c4c

To not mix intree references and source project

Add new flag to Monitor

eb995fb

This replaces the previous context base and makes it possible to disable throw for blocking calls in JS interop

Merge branch 'main' into pr-wasm-mt-throw-from-blocking-wait

5887749

pavelsavara reviewed Jan 19, 2024

View reviewed changes

...s/System.Private.CoreLib/src/System/Runtime/InteropServices/JavaScript/JSProxyContextBase.cs Outdated Show resolved Hide resolved

...rvices.JavaScript/src/System/Runtime/InteropServices/JavaScript/Interop/JavaScriptExports.cs Outdated Show resolved Hide resolved

radekdoulik added 2 commits January 19, 2024 16:44

Remove old file

4009ae3

Changes from unsaved file

dd82372

pavelsavara reviewed Jan 19, 2024

View reviewed changes

...rvices.JavaScript/src/System/Runtime/InteropServices/JavaScript/Interop/JavaScriptExports.cs Show resolved Hide resolved

radekdoulik added 2 commits January 19, 2024 16:55

Merge remote-tracking branch 'remotes/origin/main' into pr-wasm-mt-th…

e77f866

…row-from-blocking-wait

Wrap another blocking wait in JS interop

5c95514

pavelsavara mentioned this pull request Jan 30, 2024

[browser] Use standard xunit test runner on multithreaded runtime #91317

Merged

build-analysis bot mentioned this pull request Jan 30, 2024

[browser][MT] Unhandled JS error #97546

Closed

radekdoulik added 2 commits January 30, 2024 17:16

Update the active issue and don't use define

7f067df

Merge branch 'main' into pr-wasm-mt-throw-from-blocking-wait

d3a68e9

build-analysis bot mentioned this pull request Jan 30, 2024

slow macOS - "##[error]The job running on agent Azure Pipelines 9 ran longer than the maximum time of 60 minutes." dotnet/dnceng#1883

Open

3 tasks

build-analysis bot mentioned this pull request Jan 31, 2024

Assertion failed 'hwintrinsicChild->isContained()' in 'System.Numerics.Tensors.TensorPrimitives+ScaleBOperator #97688

Closed

radekdoulik marked this pull request as ready for review February 1, 2024 07:20

radekdoulik requested review from lewing, thaystg, ilonatommy and marek-safar as code owners February 1, 2024 07:20

pavelsavara approved these changes Feb 1, 2024

View reviewed changes

radekdoulik merged commit 9b7aa3d into dotnet:main Feb 1, 2024
192 of 206 checks passed

pavelsavara reviewed Feb 1, 2024

View reviewed changes

pavelsavara mentioned this pull request Feb 1, 2024

[browser][MT] JSSynchronizationContext_Send_Post_Items_Cancellation #97832

Merged

radekdoulik added a commit to radekdoulik/runtime that referenced this pull request Feb 5, 2024

Update solution files after changes in dotnet#97052

8e7528b

The project files have now diferent dependencies

radekdoulik mentioned this pull request Feb 5, 2024

Update solution files after projects changes #97969

Merged

github-actions bot locked and limited conversation to collaborators Mar 3, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[wasm][mt] throw from blocking wait on JS interop threads #97052

[wasm][mt] throw from blocking wait on JS interop threads #97052

radekdoulik commented Jan 16, 2024 •

edited by pavelsavara

Loading

dotnet-issue-labeler bot commented Jan 16, 2024

radekdoulik commented Jan 18, 2024

radekdoulik commented Jan 19, 2024

pavelsavara commented Jan 19, 2024

azure-pipelines bot commented Jan 30, 2024

radekdoulik commented Jan 30, 2024

azure-pipelines bot commented Jan 30, 2024

pavelsavara commented Jan 31, 2024

pavelsavara commented Jan 31, 2024

pavelsavara commented Jan 31, 2024

radekdoulik commented Feb 1, 2024

pavelsavara Feb 1, 2024

pavelsavara Feb 1, 2024

pavelsavara Feb 1, 2024 •

edited

Loading

pavelsavara commented Feb 1, 2024

		signal.Wait();
		Monitor.ThrowOnBlockingWaitOnJSInteropThread = threadFlag;

[wasm][mt] throw from blocking wait on JS interop threads #97052

[wasm][mt] throw from blocking wait on JS interop threads #97052

Conversation

radekdoulik commented Jan 16, 2024 • edited by pavelsavara Loading

dotnet-issue-labeler bot commented Jan 16, 2024

radekdoulik commented Jan 18, 2024

radekdoulik commented Jan 19, 2024

pavelsavara commented Jan 19, 2024

azure-pipelines bot commented Jan 30, 2024

radekdoulik commented Jan 30, 2024

azure-pipelines bot commented Jan 30, 2024

pavelsavara commented Jan 31, 2024

pavelsavara commented Jan 31, 2024

pavelsavara commented Jan 31, 2024

radekdoulik commented Feb 1, 2024

pavelsavara Feb 1, 2024

Choose a reason for hiding this comment

pavelsavara Feb 1, 2024

Choose a reason for hiding this comment

pavelsavara Feb 1, 2024 • edited Loading

Choose a reason for hiding this comment

pavelsavara commented Feb 1, 2024

radekdoulik commented Jan 16, 2024 •

edited by pavelsavara

Loading

pavelsavara Feb 1, 2024 •

edited

Loading