TestExecutionManager Task.WhenAll loses its source tasks to GC

[NinoFloris]

Under parallel execution, MSTest's TestExecutionManager awaits its worker tasks with await Task.WhenAll(tasks) starting from #4586, and never references tasks again. Since .NET 8 the non-generic Task.WhenAll promise no longer roots its source tasks (dotnet/runtime#81065), so the only thing keeping the source tasks alive is that tasks local. Roslyn however drops it from the state machine because it's not read after the await.

If a GC runs while a test task is suspended in an await chain that has no other external root, the incomplete task is collected, its completion can never propagate, and the WhenAll stays pending forever. The runner hangs with no exception and no test failure.

It's timing-dependent (needs a GC in a specific window) so it's rare, which is what makes the hang very nasty. The symptom is far from the cause, and the GC also deletes the evidence you'd use to diagnose it.

Repro (should reproduce on anything newer than .NET 8.x):
https://gist.github.com/NinoFloris/82233469bf111e6c8d6e6d0e148e06f4

The fix is to make sure TestExecutionManager keeps the tasks alive past the await, GC.Keepalive is the easiest way.

IMO this exact issue should be something that makes the runtime team reconsider the 8.0 behavior change. The Task<T> overload sits on the exact same method name unaffected by this, precisely because it roots the tasks to be able to read their results.

It is a massive footgun in the way the api differs between overloads, given how the language and the GC interact here.

[Evangelink]

Thanks for the detailed bug report @NinoFloris. I have asked for a consultancy from some runtime experts as to how we should proceed forward with this.

[Youssef1313]

I'm tending to read this as a runtime regression/bug?

[Evangelink]

Quick update — preliminary info from the .NET runtime experts (they'll dig further to confirm whether there's an actual bug here or whether this is intended behavior).

The current take is that the repro itself is flawed, and doesn't isolate the TestExecutionManager scenario it's meant to demonstrate. The problem is in how it creates and completes the worker tasks.

In the repro, the only thing that ever calls Signal() (to complete the worker tasks) is the deferred continuation in ScheduleDeferredSignal, and it holds the sources list through a WeakReference:

var weak = new WeakReference<List<ValueTaskCompletionSource>>(sources);
_ = Task.Delay(500).ContinueWith(static (_, state) =>
{
    if (((WeakReference<...>)state!).TryGetTarget(out var alive))
        foreach (var s in alive)
            s.Signal();
}, weak);

So when GC reclaims sources, it doesn''t just drop the task roots — it reclaims the signaller itself. After that, the worker tasks were never going to complete, regardless of Task.WhenAll. The "permanent strand" is really "the only thing that could complete these tasks was collected."

Tasks are expected to complete even when nothing else keeps them alive — that''s how ordinary fire-and-forget tasks work. If you change the repro so the completion is rooted the way a real fire-and-forget task is (strong reference instead of WeakReference):

_ = Task.Delay(500).ContinueWith(static (_, state) =>
{
    foreach (var s in (List<ValueTaskCompletionSource>)state!)
        s.Signal();
}, sources);

...then WhenAll completes on every run, even with sources dropped across the await and a forced Gen-2 GC in between. I ran this 5x and it completed True every time.

This doesn''t by itself prove the original concern (GC dropping Task.WhenAll''s source-task roots since .NET 8 / dotnet/runtime#81065) is a non-issue inside TestExecutionManager — only that this particular repro conflates that root-dropping behavior with tasks whose completion path was also weakly held. A faithful repro would need worker tasks that are guaranteed to complete on their own (rooted independently of the test''s locals), while the non-generic Task.WhenAll array is their sole root.

The runtime folks will keep investigating to determine whether there''s a real footgun in the WhenAll/GC interaction worth a fix, or whether this is working as intended. Will report back.

[Youssef1313]

@jeffhandley Here is a simpler snippet showing a behavior difference in runtime:

using System.Diagnostics;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;

Console.WriteLine($"Runtime: {RuntimeInformation.FrameworkDescription}");
await RunAsync();

static async Task RunAsync()
{
    Task all = Build();

    Collect();
    
    var sw = Stopwatch.StartNew();
    Task winner = await Task.WhenAny(all, Task.Delay(TimeSpan.FromSeconds(5)));
    if (winner == all)
        Console.WriteLine($"SUCCESS: WhenAll completed in {sw.ElapsedMilliseconds} ms - tasks kept alive by WhenAll.");
    else
        Console.WriteLine("FAILURE: WhenAll didn't complete in 5 seconds - its tasks were collected while pending.");
}

[MethodImpl(MethodImplOptions.NoInlining)]
static Task Build()
{
    const int taskCount = 8;

    var completer = new Completer();
    List<Task> tasks = new(taskCount);
    for (int i = 0; i < taskCount; i++)
    {
        var tcs = new TaskCompletionSource(completer);
        completer.Add(tcs);
        tasks.Add(tcs.Task);
    }

    var weak = new WeakReference<Completer>(completer);
    new Thread(() =>
    {
        Thread.Sleep(700);
        if (weak.TryGetTarget(out Completer? live))
        {
            live.CompleteAll();
        }
    })
    { IsBackground = true }.Start();

    return Task.WhenAll(tasks);
}

[MethodImpl(MethodImplOptions.NoInlining)]
static void Collect()
{
    for (int i = 0; i < 3; i++)
    {
        GC.Collect();
        GC.WaitForPendingFinalizers();
    }
}

sealed class Completer
{
    private readonly List<TaskCompletionSource> _sources = new();

    public void Add(TaskCompletionSource tcs) => _sources.Add(tcs);

    public void CompleteAll()
    {
        foreach (TaskCompletionSource tcs in _sources)
        {
            tcs.TrySetResult();
        }
    }
}
 

Output on .NET 10:

Runtime: .NET 10.0.9
FAILURE: WhenAll didn't complete in 5 seconds - its tasks were collected while pending.

And on .NET 6:

Runtime: .NET 6.0.36
SUCCESS: WhenAll completed in 714 ms - tasks kept alive by WhenAll.

I haven't tried .NET 7, 8, and 9. But I expect that .NET 6 and 7 matches together, and 8, 9, and 10 matches together.

I couldn't find a repro that doesn't involve some usage of a weak reference.

I'm not sure if this would be considered a bug in user code, even if it's a behavioral change from runtime, or if it would be seen as a runtime regression.

Generally speaking, Task.WhenAll generally doesn't need to hold the tasks in memory as long as the thing that will complete the task in the future is held in memory, which is not the case for this repro.

[NinoFloris]

I couldn't find a repro that doesn't involve some usage of a weak reference.

Yes and after reasoning through it again, that makes sense too:

• Either the async chain is running and has a natural stack root.
• Or it should be rooted somewhere that will wake it: an I/O registration, a dispatch, a timer etc.

My original assumption was that my test that uncovered this issue completed normally. But a normally completing task must fall in one of those buckets, and it wasn't in either.

So I took another look at the test this repro was built from. I added a strong root on the test's object graph with a GCHandle, and was able to reproduce the hang under stress loops. The root on the test let me trace the issue back to a lost wakeup in custom async logic (a rare race). As the wake was lost the completer let go of the source and the continuation would have become an unrooted cycle without the GCHandle.

To be clear on what I've claimed: There is no isolated bug in Task.WhenAll, there is only a behavior change. It creates a footgun in combination with its common usage pattern. It breaks the ability to diagnose issues that present as async 'hangs'. These don't only happen due to a lost wakeup: a gc hole or other native <-> managed issues could result in the same behavior.

MSTest shows a pending test and all the data to diagnose it via a dump is collected by the GC. Instead of a test process with the suspended await chain sitting in the heap, you get nothing. So for that reason alone the GC.Keepalive seems worthwhile for a test framework.

I'm less convinced of my take on the broader Task.WhenAll change than before. Given I'm the first to raise this - anywhere - (though it's likely that most people are unable to diagnose it and just move on) you could conclude it's OK to keep the array out of the promise, and take the trade: rare need for diagnosability for perf. The overload asymmetry on this still stings though.

[jkotas]

behavior difference in runtime:

We see occasional reports of behavior differences like this where code accidently depends on objects being rooted longer than strictly required. These behavior differences can be caused by number of factors - timing of GC, JIT optimizations, or changes in libraries. They are by-design. If the user code breaks due to them, it is a bug in the user code.

So for that reason alone the GC.Keepalive seems worthwhile for a test framework.

The downside is that this extra GC.Keepalive can hide issues in the code being tested.

[NinoFloris]

The downside is that this extra GC.Keepalive can hide issues in the code being tested.

True, a gc hole could stay hidden that way. I would fully agree it's a wash if not for the fact xunit and the other runners all use the Task.WhenAll<T> overload that does strongly root. Additionally I think that for a test framework it's not unreasonable to expect it to hold a strong ref to each test it has spun up.

[jkotas]

Terminology nit:

I do not see how this can hide GC holes. The GC hole definition is at https://github.com/dotnet/runtime/blob/main/docs/coding-guidelines/clr-code-guide.md#211-how-gc-holes-are-created .

It can hide other types of bugs where the code accidently depends on objects being rooted longer than strictly required. These bugs are not GC holes.