Description
openedon May 31, 2024
Lately, we have discovered that parallelism in xUnit doesn't mean: max X test cases running at the same time, but "X threads executing test cases concurrently", and it was causing us to get timeout on some tests, due to those worker threads on xUnit were blocked by RemoteExecutor.Invoke(..).Dispose();
.
So in some cases, while we're awaiting for some method (in our case it was HttpClient.SendAsync
), that worker thread is jumping back to xUnit thread pool and xUnit execute another test, if that test includes some Sync operation we're blocked until that thread finishes the test.
Here is an abstract class for repro (thanks to @rzikm):
public abstract class BaseTest
{
public bool SyncWait { get; set; }
private async Task DoRun()
{
var sw = Stopwatch.StartNew();
if (SyncWait)
{
Thread.Sleep(1000);
}
else
{
await Task.Delay(1000);
}
sw.Stop();
sw.Restart();
await Task.Delay(10);
sw.Stop();
Assert.True(sw.ElapsedMilliseconds < 100, $"Second delay took {sw.ElapsedMilliseconds}ms");
}
[Fact]
public Task Test1() => DoRun();
[Fact]
public Task Test2() => DoRun();
[Fact]
public Task Test3() => DoRun();
}
You can create several derived test classes from this abstract class and set half of their SyncWait
to true
, and you should see the message in some cases like: Second delay took 1000+ms
.
To avoid blocking on those threads we're proposing to add DisposeAsync
to RemoteInvokeHandle
.
We're currently adding this as extension
method (dotnet/runtime#102699) to decrease CI noise on our tests, but it would be nice to have a proper DisposeAsync
.
/cc @dotnet/ncl