dotnet / roslyn

The Roslyn .NET compiler provides C# and Visual Basic languages with rich code analysis APIs.
https://docs.microsoft.com/dotnet/csharp/roslyn-sdk/
MIT License
18.92k stars 4.02k forks source link

Remove JTF.Run for razor mapping #47909

Open dibarbet opened 3 years ago

dibarbet commented 3 years ago

Once https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1216657 is fixed, we should remove the places where we added JTF.Run in https://github.com/dotnet/roslyn/pull/47104#discussion_r491255962

sharwell commented 3 years ago

I'm not convinced the root cause analysis here is correct. Changing the code from a direct call to running inside a Task.Run (which the code did even before #47104) is a standard way to ensure ConfigureAwait(true) doesn't produce the described behavior.

dibarbet commented 3 years ago

@sharwell Task.Run does work here, as well as JTF.Run. From discussing with Jason we decided to switch both to JTF.Run to be safe in case anyone else was trying to switch to the main thread underneath us.

sharwell commented 3 years ago

If that's the case, can we close this issue as By Design (an alternative was available, but intentionally not used)?

dibarbet commented 3 years ago

This issue is tracking us switching to just WaitAndGetResult without task.run or jtf.run, which should be possible once https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1216657 is fixed

sharwell commented 3 years ago

LSP is external code which does not adhere to Roslyn's (unique) policy regarding UI thread dependencies. External code is broadly allowed to assume that callers will not use Task.Result, so even if there is a point in time where WaitAndGetResult works we would not be able to assume it remains true going forward.