Skip to content
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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 23 commits into from
Feb 1, 2024
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
b049c6a
[wasm][mt] throw from blocking wait on JS interop threads
radekdoulik Jan 16, 2024
150a177
Fix typo
radekdoulik Jan 17, 2024
a88e493
Add JSProxyContextBase conditionally
radekdoulik Jan 17, 2024
61b797c
Fix non-mt browser build
radekdoulik Jan 17, 2024
1ed5faa
Fix tests build
radekdoulik Jan 17, 2024
9d24021
Merge branch 'main' into pr-wasm-mt-throw-from-blocking-wait
radekdoulik Jan 17, 2024
70ad3a6
Merge remote-tracking branch 'remotes/origin/main' into pr-wasm-mt-th…
radekdoulik Jan 19, 2024
dbe7c4c
Do not add JS interop project reference
radekdoulik Jan 19, 2024
eb995fb
Add new flag to Monitor
radekdoulik Jan 19, 2024
5887749
Merge branch 'main' into pr-wasm-mt-throw-from-blocking-wait
radekdoulik Jan 19, 2024
4009ae3
Remove old file
radekdoulik Jan 19, 2024
dd82372
Changes from unsaved file
radekdoulik Jan 19, 2024
e77f866
Merge remote-tracking branch 'remotes/origin/main' into pr-wasm-mt-th…
radekdoulik Jan 19, 2024
5c95514
Wrap another blocking wait in JS interop
radekdoulik Jan 19, 2024
e1af48a
Do not reference src/System.Runtime.InteropServices.JavaScript.csproj
radekdoulik Jan 23, 2024
9ec32c1
Disable failing test with blocking Wait
radekdoulik Jan 29, 2024
91508ac
Merge branch 'main' into pr-wasm-mt-throw-from-blocking-wait
radekdoulik Jan 29, 2024
4f74e02
Update the test condition
radekdoulik Jan 30, 2024
00e6b4b
Add missing line after conflict resolution
radekdoulik Jan 30, 2024
1dd3af2
Fix build
radekdoulik Jan 30, 2024
5643036
Merge branch 'main' into pr-wasm-mt-throw-from-blocking-wait
radekdoulik Jan 30, 2024
7f067df
Update the active issue and don't use define
radekdoulik Jan 30, 2024
d3a68e9
Merge branch 'main' into pr-wasm-mt-throw-from-blocking-wait
radekdoulik Jan 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 15 additions & 7 deletions src/libraries/System.Net.Http/src/System.Net.Http.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -446,37 +446,45 @@

<ItemGroup>
<Reference Include="Microsoft.Win32.Primitives" />
<Reference Include="System.Collections" />
<Reference Include="System.Collections.Concurrent" />
<Reference Include="System.Collections.NonGeneric" />
<Reference Include="System.Console" Condition="'$(Configuration)' == 'Debug'" />
<Reference Include="System.Diagnostics.DiagnosticSource" />
<Reference Include="System.Diagnostics.Tracing" />
<Reference Include="System.IO.Compression" />
<Reference Include="System.Memory" />
<Reference Include="System.Net.NameResolution" />
<Reference Include="System.Net.NetworkInformation" />
<Reference Include="System.Net.Primitives" />
<Reference Include="System.Net.Quic" />
<Reference Include="System.Net.Security" />
<Reference Include="System.Net.Sockets" />
<Reference Include="System.Runtime" />
<Reference Include="System.Runtime.InteropServices" />
<Reference Include="System.Security.Claims" Condition="'$(TargetPlatformIdentifier)' == 'windows'" />
<Reference Include="System.Security.Cryptography" />
<Reference Include="System.Security.Principal.Windows" />
<Reference Include="System.Threading" />
<Reference Include="System.Threading.Channels" />
<Reference Include="System.Threading.ThreadPool" />
<Reference Include="System.IO.Compression.Brotli" />
</ItemGroup>

<ItemGroup Condition="'$(TargetPlatformIdentifier)' != 'browser'">
<Reference Include="System.Collections" />
<Reference Include="System.Collections.Concurrent" />
<Reference Include="System.Diagnostics.Tracing" />
<Reference Include="System.Memory" />
<Reference Include="System.Runtime" />
<Reference Include="System.Runtime.InteropServices" />
<Reference Include="System.Threading" />
</ItemGroup>

<ItemGroup Condition="'$(TargetPlatformIdentifier)' != '' and '$(TargetPlatformIdentifier)' != 'windows' and '$(TargetPlatformIdentifier)' != 'browser'">
<Reference Include="System.Diagnostics.StackTrace" />
<Reference Include="System.Security.Cryptography" />
</ItemGroup>

<ItemGroup Condition="'$(TargetPlatformIdentifier)' == 'browser'">
<ProjectReference Include="$(CoreLibProject)" />
<ProjectReference Include="$(LibrariesProjectRoot)System.Runtime\src\System.Runtime.csproj" />
<ProjectReference Include="$(LibrariesProjectRoot)System.Threading\src\System.Threading.csproj" />
<ProjectReference Include="$(LibrariesProjectRoot)System.Private.Uri\src\System.Private.Uri.csproj" PrivateAssets="all" />
<ProjectReference Include="$(LibrariesProjectRoot)System.Collections.Concurrent\src\System.Collections.Concurrent.csproj" />
<ProjectReference Include="$(LibrariesProjectRoot)System.Runtime.InteropServices\gen\Microsoft.Interop.SourceGeneration\Microsoft.Interop.SourceGeneration.csproj" OutputItemType="Analyzer" ReferenceOutputAssembly="false" />
<ProjectReference Include="$(LibrariesProjectRoot)System.Runtime.InteropServices.JavaScript\gen\JSImportGenerator\JSImportGenerator.csproj" OutputItemType="Analyzer" ReferenceOutputAssembly="false" />
<ProjectReference Include="$(LibrariesProjectRoot)System.Runtime.InteropServices.JavaScript\src\System.Runtime.InteropServices.JavaScript.csproj" SkipUseReferenceAssembly="true" />
Expand Down
14 changes: 10 additions & 4 deletions src/libraries/System.Net.WebSockets.Client/src/System.Net.WebSockets.Client.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,23 +38,29 @@

<ItemGroup>
<Reference Include="Microsoft.Win32.Primitives" />
<Reference Include="System.Collections" />
<Reference Include="System.Collections.NonGeneric" />
<Reference Include="System.Collections.Specialized" />
<Reference Include="System.Diagnostics.Tracing" />
<Reference Include="System.Net.Primitives" />
<Reference Include="System.Net.Security" />
<Reference Include="System.Net.WebHeaderCollection" />
<Reference Include="System.Net.WebSockets" />
<Reference Include="System.Runtime" />
<Reference Include="System.Threading" />
<Reference Include="System.Net.Http" />
<Reference Include="System.Security.Cryptography" />
<Reference Include="System.Threading.Channels" Condition="'$(TargetPlatformIdentifier)' == 'browser'" />
</ItemGroup>

<ItemGroup Condition="'$(TargetPlatformIdentifier)' != 'browser'">
<Reference Include="System.Collections" />
<Reference Include="System.Runtime" />
<Reference Include="System.Threading" />
<Reference Include="System.Memory" />
</ItemGroup>

<ItemGroup Condition="'$(TargetPlatformIdentifier)' == 'browser'">
<Reference Include="System.Threading.Channels" />
<ProjectReference Include="$(CoreLibProject)" />
<ProjectReference Include="$(LibrariesProjectRoot)System.Private.Uri\src\System.Private.Uri.csproj" PrivateAssets="all" />
<ProjectReference Include="$(LibrariesProjectRoot)System.Runtime\src\System.Runtime.csproj" />
<ProjectReference Include="$(LibrariesProjectRoot)System.Runtime.InteropServices\gen\Microsoft.Interop.SourceGeneration\Microsoft.Interop.SourceGeneration.csproj" OutputItemType="Analyzer" ReferenceOutputAssembly="false" />
<ProjectReference Include="$(LibrariesProjectRoot)System.Runtime.InteropServices.JavaScript\gen\JSImportGenerator\JSImportGenerator.csproj" OutputItemType="Analyzer" ReferenceOutputAssembly="false" />
<ProjectReference Include="$(LibrariesProjectRoot)System.Runtime.InteropServices.JavaScript\src\System.Runtime.InteropServices.JavaScript.csproj" SkipUseReferenceAssembly="true" />
Expand Down
11 changes: 11 additions & 0 deletions src/libraries/System.Private.CoreLib/ref/System.Private.CoreLib.ExtraApis.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
1 change: 1 addition & 0 deletions src/libraries/System.Private.CoreLib/ref/System.Private.CoreLib.ExtraApis.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
16 changes: 4 additions & 12 deletions ...m.Runtime.InteropServices.JavaScript/src/System.Runtime.InteropServices.JavaScript.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,18 +74,10 @@
</ItemGroup>

<ItemGroup>
<Reference Include="System.Collections" />
<Reference Include="System.Memory" />
<Reference Include="System.Net.Primitives" />
<Reference Include="System.Runtime" />
<Reference Include="System.Runtime.InteropServices" />
<Reference Include="System.Runtime.Loader" />
<Reference Include="System.Threading" />
<Reference Include="System.Threading.Thread" />
<Reference Include="System.Threading.Channels" />
</ItemGroup>

<ItemGroup>
<ProjectReference Include="$(CoreLibProject)" />
<ProjectReference Include="$(LibrariesProjectRoot)System.Runtime\src\System.Runtime.csproj" />
<ProjectReference Include="$(LibrariesProjectRoot)System.Threading\src\System.Threading.csproj" />
<ProjectReference Include="$(LibrariesProjectRoot)System.Threading.Channels\src\System.Threading.Channels.csproj" />
<ProjectReference Include="$(LibrariesProjectRoot)System.Runtime.InteropServices\gen\Microsoft.Interop.SourceGeneration\Microsoft.Interop.SourceGeneration.csproj" OutputItemType="Analyzer" ReferenceOutputAssembly="false" />
<ProjectReference Include="$(LibrariesProjectRoot)System.Runtime.InteropServices.JavaScript\gen\JSImportGenerator\JSImportGenerator.csproj" OutputItemType="Analyzer" ReferenceOutputAssembly="false" />
</ItemGroup>
Expand Down
3 changes: 3 additions & 0 deletions ...ces.JavaScript/src/System/Runtime/InteropServices/JavaScript/Interop/JavaScriptExports.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
radekdoulik marked this conversation as resolved.
Show resolved Hide resolved
}
#endif
var callback = holder.Callback!;
Expand Down
4 changes: 4 additions & 0 deletions ...ices.JavaScript/src/System/Runtime/InteropServices/JavaScript/JSSynchronizationContext.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm re-thinking if this should be allowed after all. Because that could call code on the other thread, which would be long blocking.

At the moment, this is necessary for the HTTP/WS clients to work across threads. But now I will change the implementation to use emscripten instead of JSSynchronizationContext to make that dispatch.
It will also have the same problem, tho.

So I guess we merge this for now and I will improve it bit later.

Monitor.ThrowOnBlockingWaitOnJSInteropThread = false;
signal.Wait();
Monitor.ThrowOnBlockingWaitOnJSInteropThread = threadFlag;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

@pavelsavara pavelsavara Feb 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll will change that on my PR #97832


if (_isCancellationRequested)
{
Expand Down
8 changes: 7 additions & 1 deletion ...teropServices.JavaScript.UnitTests/System.Runtime.InteropServices.JavaScript.Tests.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,13 @@
<WasmExtraFilesToDeploy Include="System\Runtime\InteropServices\JavaScript\JavaScriptTestHelper.mjs" />
<WasmExtraFilesToDeploy Include="System\Runtime\InteropServices\JavaScript\SecondRuntimeTest.js" />
<WasmExtraFilesToDeploy Include="System\Runtime\InteropServices\JavaScript\timers.mjs" />
<ProjectReference Include="$(CoreLibProject)" />
<ProjectReference Include="$(LibrariesProjectRoot)System.Private.Uri\src\System.Private.Uri.csproj" PrivateAssets="all" />
<ProjectReference Include="$(LibrariesProjectRoot)System.Collections\src\System.Collections.csproj" />
<ProjectReference Include="$(LibrariesProjectRoot)System.Runtime\src\System.Runtime.csproj" />
<ProjectReference Include="$(LibrariesProjectRoot)System.Memory\src\System.Memory.csproj" />
<ProjectReference Include="$(LibrariesProjectRoot)System.Threading\src\System.Threading.csproj" />
<ProjectReference Include="$(LibrariesProjectRoot)System.Threading.Thread\src\System.Threading.Thread.csproj" />
<ProjectReference Include="$(LibrariesProjectRoot)System.Runtime.InteropServices.JavaScript\src\System.Runtime.InteropServices.JavaScript.csproj" SkipUseReferenceAssembly="true" />
</ItemGroup>
<ItemGroup Condition="'$(FeatureWasmThreads)' != 'true'">
Expand All @@ -46,6 +53,5 @@
<None Include="System\Runtime\InteropServices\JavaScript\test.json" />
<WasmExtraFilesToDeploy Include="System\Runtime\InteropServices\JavaScript\WebWorkerTestHelper.mjs" />
<WasmExtraFilesToDeploy Include="System\Runtime\InteropServices\JavaScript\test.json" />
<ProjectReference Include="$(LibrariesProjectRoot)System.Runtime.InteropServices.JavaScript\src\System.Runtime.InteropServices.JavaScript.csproj" SkipUseReferenceAssembly="true" />
</ItemGroup>
</Project>
19 changes: 19 additions & 0 deletions ...pServices.JavaScript.UnitTests/System/Runtime/InteropServices/JavaScript/WebWorkerTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -411,6 +411,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)
{
Expand Down
16 changes: 16 additions & 0 deletions ...ces.JavaScript.UnitTests/System/Runtime/InteropServices/JavaScript/WebWorkerTestHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<PlatformNotSupportedException>(exception);
break;
case ExecutorType.NewThread:
case ExecutorType.ThreadPool:
Assert.Null(exception);
break;
}
}

public void AssertInteropThread()
{
switch (Type)
Expand Down
4 changes: 4 additions & 0 deletions src/libraries/System.Threading/src/CompatibilitySuppressions.Threading.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,4 +100,8 @@
<DiagnosticId>CP0014</DiagnosticId>
<Target>M:System.Threading.Monitor.Wait(System.Object):[T:System.Runtime.Versioning.UnsupportedOSPlatformAttribute]</Target>
</Suppression>
<Suppression>
<DiagnosticId>CP0002</DiagnosticId>
<Target>F:System.Threading.Monitor.ThrowOnBlockingWaitOnJSInteropThread</Target>
</Suppression>
</Suppressions>
11 changes: 11 additions & 0 deletions src/mono/System.Private.CoreLib/src/System/Threading/Monitor.Mono.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

if (ThrowOnBlockingWaitOnJSInteropThread)
{
throw new PlatformNotSupportedException("blocking Wait is not supported on the JS interop threads.");
}
#endif
return ObjWait(millisecondsTimeout, obj);
}

Expand Down
1 change: 1 addition & 0 deletions src/mono/browser/debugger/DebuggerTestSuite/SteppingTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
4 changes: 0 additions & 4 deletions src/mono/browser/debugger/tests/debugger-test/debugger-test.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,9 +119,5 @@
</ItemGroup>
</Target>

<ItemGroup>
<ProjectReference Include="$(LibrariesProjectRoot)System.Runtime.InteropServices.JavaScript\src\System.Runtime.InteropServices.JavaScript.csproj" SkipUseReferenceAssembly="true"/>
</ItemGroup>

<Import Project="$(BrowserProjectRoot)\build\WasmApp.InTree.targets" />
</Project>
4 changes: 0 additions & 4 deletions src/tests/FunctionalTests/WebAssembly/Directory.Build.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,6 @@
<ExpectedExitCode>42</ExpectedExitCode>
</PropertyGroup>

<ItemGroup>
<ProjectReference Include="$(LibrariesProjectRoot)System.Runtime.InteropServices.JavaScript\src\System.Runtime.InteropServices.JavaScript.csproj" SkipUseReferenceAssembly="true"/>
</ItemGroup>

<Import Project="$([MSBuild]::GetPathOfFileAbove(Directory.Build.props, '$(MSBuildThisFileDirectory)..'))" />

<ItemGroup Condition="'$(IsWasmTestProject)' != 'false'">
Expand Down
Loading