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.

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

Blazor client developers can use Parallel.For in their applications to share more code with server #43411

Closed
Tracked by #43767
andersson09 opened this issue Oct 14, 2020 · 45 comments
Assignees
Labels
arch-wasm WebAssembly architecture area-System.Threading.Tasks Priority:2 Work that is important, but not critical for the release User Story A single user-facing feature. Can be grouped under an epic.
Milestone

Comments

@andersson09
Copy link

andersson09 commented Oct 14, 2020

Description

Hi,

So the Roslyn compiler is kind of working in blazor wasm with the latest version (3.8.0-4.final) which is great! Appears the hashing issues are fixed. However, now I find there is a strange bug whereby I receive a multi threading exception under the following scenario:

  • Dotnet run
  • Load web page in browser
  • Run compile (works fine)
  • Refresh page in browser
  • Run compile (exception)

Strangely it works if I close the original tab and open a new one.

Thanks

Configuration

  • .NET 5.0.100-rc.2.20479.15
  • Blazor wasm
  • Microsoft.CodeAnalysis 3.8.0-4.final
  • Chrome

Regression?

Last time Roslyn was working in the browser was with Blazor Webassembly 3.2.1 and Microsoft.CodeAnalysis 3.6.0

Other information

System.PlatformNotSupportedException: Cannot wait on monitors on this runtime.
   at System.Threading.Monitor.ObjWait(Boolean exitContext, Int32 millisecondsTimeout, Object obj)
   at System.Threading.Monitor.Wait(Object obj, Int32 millisecondsTimeout, Boolean exitContext)
   at System.Threading.Monitor.Wait(Object obj, Int32 millisecondsTimeout)
   at System.Threading.ManualResetEventSlim.Wait(Int32 millisecondsTimeout, CancellationToken cancellationToken)
   at System.Threading.Tasks.Task.SpinThenBlockingWait(Int32 millisecondsTimeout, CancellationToken cancellationToken)
   at System.Threading.Tasks.Task.InternalWaitCore(Int32 millisecondsTimeout, CancellationToken cancellationToken)
   at System.Threading.Tasks.Task.InternalWait(Int32 millisecondsTimeout, CancellationToken cancellationToken)
   at System.Threading.Tasks.Task.Wait(Int32 millisecondsTimeout, CancellationToken cancellationToken)
   at System.Threading.Tasks.Task.Wait()
   at System.Threading.Tasks.TaskReplicator.Replica.Wait()
   at System.Threading.Tasks.TaskReplicator.Run[RangeWorker](ReplicatableUserAction`1 action, ParallelOptions options, Boolean stopOnFirstFailure)
   at System.Threading.Tasks.Parallel.ForWorker[Object](Int32 fromInclusive, Int32 toExclusive, ParallelOptions parallelOptions, Action`1 body, Action`2 bodyWithState, Func`4 bodyWithLocal, Func`1 localInit, Action`1 localFinally)
   at System.Threading.Tasks.Parallel.For(Int32 fromInclusive, Int32 toExclusive, ParallelOptions parallelOptions, Action`1 body)
   at Microsoft.CodeAnalysis.CSharp.CSharpCompilation.GetDiagnostics(CompilationStage stage, Boolean includeEarlierStages, DiagnosticBag diagnostics, CancellationToken cancellationToken)
   at Microsoft.CodeAnalysis.CSharp.CSharpCompilation.GetDiagnostics(CompilationStage stage, Boolean includeEarlierStages, CancellationToken cancellationToken)
   at Microsoft.CodeAnalysis.CSharp.CSharpCompilation.CompileMethods(CommonPEModuleBuilder moduleBuilder, Boolean emittingPdb, Boolean emitMetadataOnly, Boolean emitTestCoverageData, DiagnosticBag diagnostics, Predicate`1 filterOpt, CancellationToken cancellationToken)
   at Microsoft.CodeAnalysis.Compilation.Emit(Stream peStream, Stream metadataPEStream, Stream pdbStream, Stream xmlDocumentationStream, Stream win32Resources, IEnumerable`1 manifestResources, EmitOptions options, IMethodSymbol debugEntryPoint, Stream sourceLinkStream, IEnumerable`1 embeddedTexts, CompilationTestData testData, CancellationToken cancellationToken)
   at Microsoft.CodeAnalysis.Compilation.Emit(Stream peStream, Stream pdbStream, Stream xmlDocumentationStream, Stream win32Resources, IEnumerable`1 manifestResources, EmitOptions options, IMethodSymbol debugEntryPoint, Stream sourceLinkStream, IEnumerable`1 embeddedTexts, Stream metadataPEStream, CancellationToken cancellationToken)
@danmoseley danmoseley added the arch-wasm WebAssembly architecture label Oct 14, 2020
@danmoseley
Copy link
Member

I think this is by design -- Roslyn uses mechanisms that rely on mutexes, and those aren't supported on Blazor.
@marek-safar given there's only one thread in WASM, as I understand it, is there any kind of cooperative multithreading support, such that a mutex could work?

@andersson09
Copy link
Author

andersson09 commented Oct 14, 2020

@danmosemsft But why would it work on first page load? Also to note it works when you close the original tab and create a new one.

@danmoseley
Copy link
Member

I am guessing that if no task is already in progress, it has a special path that does not bother to check the mutex.

@maryamariyan maryamariyan added the untriaged New issue has not been triaged by the area owner label Oct 14, 2020
@marek-safar
Copy link
Contributor

marek-safar commented Oct 15, 2020

Although System.Threading.Monitor.Wait is unsupported API on wasm I'm reluctant to simply mark the issue as by design. We have Parallel.For implementation which does not work on wasm with no warning to the developers which is not great.

There two different solutions I can think of

  • We could fix Parallel.For implementation not to use any threads in a single thread environment and this config option for single CPU setups
  • We could mark the API as unsupported on single thread platforms (this is easy for wasm but what about others?)

@jeffhandley @buyaa-n I'm concern that #43363 did catch this issue. Could you please investigate why?

@ghost
Copy link

ghost commented Oct 15, 2020

Tagging subscribers to this area: @tarekgh
See info in area-owners.md if you want to be subscribed.

@marek-safar marek-safar added area-System.Threading.Tasks and removed area-VM-threading-mono untriaged New issue has not been triaged by the area owner labels Oct 15, 2020
@ghost
Copy link

ghost commented Oct 15, 2020

Tagging subscribers to this area: @tarekgh
See info in area-owners.md if you want to be subscribed.

@andersson09
Copy link
Author

andersson09 commented Oct 15, 2020

@marek-safar I think it would be beneficial to go with the first option (of course I would say that :) ). The roslyn api is so close to working perfectly under wasm and I think a significant number of users are using this api within the browser. Would marking the API as unsupported mean even the features that were working wouldn't work?

Perhaps adding an option as an input parameter to disable multithreading could also work?

@marek-safar
Copy link
Contributor

marking the API as unsupported mean even the features that were working wouldn't work

Marking API as unsupported would push the responsibility not to call this API to developers which is not a great option in this case (everyone you'd need to think about the else case in if Environment.ProcessorCount != 1)

@stephentoub
Copy link
Member

We could fix Parallel.For implementation not to use any threads

If Environment.ProcessorCount == 1, Parallel.For should not end up using other threads. You see otherwise?

@stephentoub
Copy link
Member

stephentoub commented Oct 15, 2020

Oops, nevermind, I was thinking of PLINQ.

It can make sense for Parallel.For to use multiple threads even on a single core. The difference with wasm/blazor is it's not just single core but also single thread.

@marek-safar
Copy link
Contributor

I was thinking of PLINQ.

Heh, me too. Tweaked my comment to focus on a single thread environment

@akoeplinger
Copy link
Member

I agree we should fix Parallel.For to not attempt to use multiple threads.

@stephentoub
Copy link
Member

I suspect this is related to #2078 as well, and that a fix to that would make this error go away without further changes to Parallel.For to try to special-case a system that only ever has a single thread.

@danmoseley
Copy link
Member

Do we have API to detect whether only 1 thread is possible? (Environment.SingleThreaded or so)

@stephentoub
Copy link
Member

Nope

@jkotas
Copy link
Member

jkotas commented Oct 15, 2020

You can always try/catch PNSE.

@marek-safar
Copy link
Contributor

Do we have API to detect whether only 1 thread is possible?

What about ThreadPool.GetMaxThreads?

@stephentoub
Copy link
Member

What about ThreadPool.GetMaxThreads?

In a console app, as an example, if GetMaxThreads said there was one worker thread, that still means the app has at least two threads on which it can execute code: the main thread and that single thread pool thread. With Blazor/wasm, they're one in the same.

@akoeplinger
Copy link
Member

For #38690 what we did was add an internal bool Thread.IsThreadStartSupported that is hardcoded to false on Browser and true everywhere else. It's internal to System.Private.Corelib.dll but I guess we can just duplicate that in System.Threading.Tasks.Parallel.

@marek-safar
Copy link
Contributor

if GetMaxThreads said there was one worker thread, that still means the app has at least two threads on which it can execute code: the main thread and that single thread pool thread

The app can have 100 threads but if TP is limited to single thread System.Threading.Tasks.Parallel.For should probably be optimized for sequential execution

@stephentoub
Copy link
Member

The app can have 100 threads but if TP is limited to single thread System.Threading.Tasks.Parallel.For should probably be optimized for sequential execution

Parallel.For uses the current thread. If you run it from the main thread and there's also a thread pool thread, it should use both.

@andersson09
Copy link
Author

Hey does anyone have a rough ETA / update for this? Will it be expected to be fixed for main release coming up?

@lewing
Copy link
Member

lewing commented Nov 15, 2020

@andersson09 if you can easily create a sample that failed with rc2 and the older roslyn I would definitely still be interested in looking at it. This path has changed a little in roslyn but not in a way that would obviously resolve what appeared to be the issue here.

@marek-safar
Copy link
Contributor

@lewing presumably this should also be trigger by some of the existing tests

@danmoseley danmoseley changed the title Blazor wasm Roslyn PlatformNotSupportedException under certain scenario Blazor wasm can fail doing Parallel.For Nov 16, 2020
@danmoseley danmoseley added the User Story A single user-facing feature. Can be grouped under an epic. label Nov 24, 2020
@danmoseley danmoseley changed the title Blazor wasm can fail doing Parallel.For Blazor wasm app should be able to use Parallel.For Nov 24, 2020
@marek-safar marek-safar changed the title Blazor wasm app should be able to use Parallel.For Blazor client developers can use Parallel.For in their applications to share more code with server Nov 27, 2020
@marek-safar marek-safar added the Priority:2 Work that is important, but not critical for the release label Nov 27, 2020
@RChrisCoble
Copy link

It almost sounds like you should just support this instead:

Real multithreading (on supported browsers) #17730

@ikeough
Copy link

ikeough commented Dec 2, 2020

Our issue is exactly the opposite of @andersson09's, and happens in a different place, but seems to have the same root cause. Our call to CSharpCompilation.CreateScriptCompilation succeeds but the first time we try to do CSharpCompilation.GetDiagnostics following the compilation call we get the Cannot wait on monitors... exception (stack trace below). The second time we execute this exact same code path, it succeeds without throwing an exception.

The solutions proposed above to "fix" Parallel.For seem like they would fix this issue as well, but it would be nice to have a work-around. I see the work-around proposed here, but that doesn't help those of us who aren't calling Parallel.For directly. Perhaps the ability to pass through the ParallelOptions for all the CSharpCompilation methods that internally use Parallel.For, would enable us to use the proposed work-around.

Cannot wait on monitors on this runtime.
blazor.webassembly.js:1    at System.Threading.Monitor.ObjWait(Boolean exitContext, Int32 millisecondsTimeout, Object obj)
blazor.webassembly.js:1    at System.Threading.Monitor.Wait(Object obj, Int32 millisecondsTimeout, Boolean exitContext)
blazor.webassembly.js:1    at System.Threading.Monitor.Wait(Object obj, Int32 millisecondsTimeout)
blazor.webassembly.js:1    at System.Threading.ManualResetEventSlim.Wait(Int32 millisecondsTimeout, CancellationToken cancellationToken)
blazor.webassembly.js:1    at System.Threading.Tasks.Task.SpinThenBlockingWait(Int32 millisecondsTimeout, CancellationToken cancellationToken)
blazor.webassembly.js:1    at System.Threading.Tasks.Task.InternalWaitCore(Int32 millisecondsTimeout, CancellationToken cancellationToken)
blazor.webassembly.js:1    at System.Threading.Tasks.Task.InternalWait(Int32 millisecondsTimeout, CancellationToken cancellationToken)
blazor.webassembly.js:1    at System.Threading.Tasks.Task.Wait(Int32 millisecondsTimeout, CancellationToken cancellationToken)
blazor.webassembly.js:1    at System.Threading.Tasks.Task.Wait()
blazor.webassembly.js:1    at System.Threading.Tasks.TaskReplicator.Replica.Wait()
blazor.webassembly.js:1    at System.Threading.Tasks.TaskReplicator.Run[RangeWorker](ReplicatableUserAction`1 action, ParallelOptions options, Boolean stopOnFirstFailure)
blazor.webassembly.js:1    at System.Threading.Tasks.Parallel.ForWorker[Object](Int32 fromInclusive, Int32 toExclusive, ParallelOptions parallelOptions, Action`1 body, Action`2 bodyWithState, Func`4 bodyWithLocal, Func`1 localInit, Action`1 localFinally)
blazor.webassembly.js:1    at System.Threading.Tasks.Parallel.For(Int32 fromInclusive, Int32 toExclusive, ParallelOptions parallelOptions, Action`1 body)
blazor.webassembly.js:1    at Microsoft.CodeAnalysis.CSharp.CSharpCompilation.GetDiagnostics(CompilationStage stage, Boolean includeEarlierStages, DiagnosticBag diagnostics, CancellationToken cancellationToken)
blazor.webassembly.js:1    at Microsoft.CodeAnalysis.CSharp.CSharpCompilation.GetDiagnostics(CompilationStage stage, Boolean includeEarlierStages, CancellationToken cancellationToken)
blazor.webassembly.js:1    at Microsoft.CodeAnalysis.CSharp.CSharpCompilation.GetDiagnostics(CancellationToken cancellationToken)
blazor.webassembly.js:1    at ElementsWasm.Pages.Model.TryCompile(String source, CSharpCompilation& compilation, Assembly& assembly, List`1& errorDiagnostics) in /Users/ikeough/Documents/ElementsWasm/Pages/Model.razor:line 143

@ivan-prodanov
Copy link

ivan-prodanov commented Mar 3, 2021

Guys, could you please help me find out how this issue was fixed? I sense this was fixed specifically and only in Blazor.

My non-blazor wasm app has the same issue as OP. If I move the code to a blazor app it works just fine. In my non-blazor wasm app I get the same error "Cannot wait on monitors on this runtime.".

Was the issue addressed in Roslyn itself? Where's the fix?

@danmoseley
Copy link
Member

@lewing ?

@marek-safar
Copy link
Contributor

The issue is not yet fixed. It's planned for .NET6 release with Priority 2 (see the labels).

@ivan-prodanov
Copy link

@marek-safar @lewing It most certainly is fixed in Blazor .NET 5, because you can use Roslyn. I've checked it, it does use Parallel.For inside GetDiagnostics (concurrentBuild option on CSharpCompilationOptions is true).

The easiest way to reproduce this without Roslyn is to call this C# code anywhere from js "Parallel.For(0, 10, i => Console.WriteLine(i));". It works only in Blazor apps.

@lewing
Copy link
Member

lewing commented Aug 17, 2021

We've landed an implementation in #57283 once @kg has opened a tracking issue for the follow-up this can be closed.

@lewing
Copy link
Member

lewing commented Aug 17, 2021

#57580

@lewing lewing closed this as completed Aug 17, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Sep 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-wasm WebAssembly architecture area-System.Threading.Tasks Priority:2 Work that is important, but not critical for the release User Story A single user-facing feature. Can be grouped under an epic.
Projects
None yet
Development

No branches or pull requests