Opting out of AsyncTestSyncContext (original) (raw)
Hi All,
Everyone today's under the impression we are done with deadlocks because of SynchronizationContexts since they were removed in ASP.NET Core.
Problem is, the same code that works perfectly in development, staging and production is causing very weird problems and even deadlocks when run inside xunit tests.
This is because xunit added 2 SynchronizationContexts:
- MaxConcurrencySyncContext for limiting the number of threads
- AsyncTestSyncContext to keep track of
async voidtests
While the former can easily be opted-out, with MaxParallelThreads=-1, with AsyncTestSyncContext since it's being used very deep inside the framework in https://github.com/xunit/xunit/blob/v2/src/xunit.execution/Sdk/Frameworks/Runners/TestInvoker.cs#L238 it seems like you simply cannot opt-out unless you override a half of the framework.
I noticed this is fixed with v3, where we only get AsyncTestSyncContext if the test method is in fact async void: https://github.com/xunit/xunit/blob/main/src/xunit.v3.core/Sdk/v3/Runners/TestInvoker.cs#L143
Personally, I would throw an exception for async void, aligning xunit with .NET's other test frameworks that do not support async void (I honestly see no advantage of having async void, probably a typo and can be fixed in a second or with a linter but many many disadvantages).
Can we please add a reasonable way of opting-out of AsyncTestSyncContext in the production V2 branch?
If investing a lot of time in it is not desirable since it's already fixed in V3, we can very easily just look at an environment variable in TestInvoker.
Bear in mind:
- Finding the root cause of these deadlocks is very difficult and time consuming
- ASP.NET Core integration tests framework shockingly is using sync over async in its code base, that making deadlocks super likely in big projects. See this thread: Tests hang from dotnet test microsoft/vstest#2080 (comment)
- Code under test should run and behave as much as possible as code outside the test, putting code inside a SynchronizationContext for tests, is a huge and impactful change.
Best,
Doron Grinzaig