Deadlock with xUnit and async

I have deadlocks on just some machines using this xUnit test.

Unfortunately this is shortened code and as the problem seems to accur randomly it is not reproducable with this code. But I hope I got everthing that is important.

A dump showed me the test it stuck. and an analysis tells me this:

1 threads are performing asynchronous work in method 'DataHandler.LogSpecialOperation(...)', but are waiting on a synchronous call to method 'System.Threading.ManualResetEventSlim.Wait(int, System.Threading.CancellationToken)'. This may cause thread pool starvation and hangs.

The test and the classes
PS: I have more than one test similar to this. Maybe they block each other.

[Fact]
public void TestDataHandlerLogging()
{
	var logger = new StubLogger<TestClass>();
	var fileWriter = new StubDumpFileWriter(); // as this is a stub: does nothing special, no file system access, ...

	var dataHandler = new DataHandler(logger, fileWriter);
	dataHandler.LogSpecialOperation(...);

	...
}

public class DataHandler
{
	private readonly object specialOperationLock = new(); // not used by the StubDumpFileWriter so should not be responsible for deadlocks in tests

	internal void LogSpecialOperation(...)
	{
		CsvDataLogger.LogCsvToFileAsync(specialOperationLock, ..., logger, dumpFileWriter)
            .GetAwaiter().GetResult();
	}
}

public class CsvDataLogger
{
    public static async Task LogCsvToFileAsync(object lockObject, ..., ILogger logger, IDumpFileWriter dumpFileWriter)
    {
        await Task.Run(() =>
        {
            dumpFileWriter.WriteCsv(lockObject, ..., logger); // as this is a stub, nothing important should happen here
        });
    }
}

What confuses me is ManualResetEventSlim.Wait in the analysis. I think I don't use it anywhere. Is it from xUnit? Is there something I have to consider when using xUnit?

As Marc suspected:

> if I was writing a testing framework, I'd intentionally design it to have such a sync context, specifically to help find incorrect usage like this

xUnit does have a one-thread-at-a-time synchronization context applied to all its tests. Not all frameworks do; older ones have prioritized backwards compatibility - MSTest never provides a context and NUnit sometimes does and sometimes doesn't. But xUnit was new enough when async came out that they placed a context on every test.

With any one-thread-at-a-time context, blocking code like what you posted could cause deadlocks (as described on my blog). It happens "randomly" because of how await behaves; it can behave synchronously (as described on my blog) if its awaitable (task) is already complete - so the deadlock will not occur if the awaited code completes too quickly.

If "DataHandler" and ILogger imply ASP.NET Core, then at runtime this kind of deadlock cannot occur because ASP.NET Core does not have a one-thread-at-a-time context. But it's still a bad idea to block anyway (as described on my blog), because it wastes a thread unnecessarily.

> I think I'll get rid of the GetAwaiter().GetResult(). Takes a bit of time but hopefully is worth it.

I must echo the sentiment of another commenter:

> This has been solved in X Logging Frameworks. Why not use them?

There are several high-quality, heavily tested logging frameworks that have solved this problem appropriately. The usual approach is to (synchronously) place log messages into a queue and occasionally flush those log messages to storage from a background thread. This way, your log statements don't have to be asynchronous all throughout your application.