Fix incorrect code generation on async iterators

[henriquewr]

Fixes #81467

The implementation of Async iterators currently overrides the value of the field _combinedTokens when calling GetAsyncEnumerator multiple times, preventing it from being disposed

I changed it so that, instead of being assigned to this instance, it is now assigned to the result instance (the instance that the method is returning), to not override the previous value

[henriquewr]

Is there a better way to do it?
I tested with reflection and the names of the fields differs from net framework (m_disposed) and net 10 (_disposed)

[jjonescz]

Consider using raw string literals (""") for new tests.

[jjonescz]

Do we need reflection? Is this bug not reproable using public APIs only?

Side question - how did you discover this bug if not in real-world code (which presumably wouldn't use reflection like this)?

[henriquewr]

Do we need reflection? Is this bug not reproable using public APIs only?

I'm not sure exactly how to check if this bug occurs without using reflection

Side question - how did you discover this bug if not in real-world code (which presumably wouldn't use reflection like this)?

Really interesting question

I was reverse engineering the IAsyncEnumerable implementation in SharpLab, saw the combinedTokens field and though it could be overriden by calling GetAsyncEnumerator multiple times

So, I wrote in c# to debug, and saw that the bug was indeed happening

Then I went to the Roslyn repo to find out how this was implemented and realized that the comments didn't really reflect what the code did, and then everything made sense

[jjonescz]

We definitely shouldn't rely on internal standard library implementation details via reflection. But it is possible to redefine standard library APIs which is especially useful for testing. For example, this seems to produce different output before and after this PR:

#pragma warning disable CS0436

using System;
using System.Collections.Generic;
using System.Runtime.CompilerServices;
using System.Threading;
using System.Threading.Tasks;

var token1 = new CancellationTokenSource();
var iterator = Create(token1.Token);
var token2 = new CancellationTokenSource();
var enumerator1 = iterator.GetAsyncEnumerator(token2.Token);
await Move(enumerator1);
var token3 = new CancellationTokenSource();
var enumerator2 = iterator.GetAsyncEnumerator(token3.Token);
await Move(enumerator2);
await Move(enumerator2);
await Move(enumerator2);

static async Task Move(IAsyncEnumerator<int> enumerator)
{
    try
    {
        if (await enumerator.MoveNextAsync())
            Console.Write(enumerator.Current);
        else
            Console.Write('f');
    }
    catch (OperationCanceledException) { Console.Write('c'); }
}

static async IAsyncEnumerable<int> Create([EnumeratorCancellation] CancellationToken cancellationToken)
{
    yield return 1;
    yield return 2;
}

namespace System.Threading
{
    public class CancellationTokenSource
    {
        public static CancellationTokenSource CreateLinkedTokenSource(CancellationToken token1, CancellationToken token2) => new();
        public CancellationToken Token => new(this);
        public void Dispose() { Console.Write('d'); }
        public void Cancel() { }
    }

    public record struct CancellationToken(CancellationTokenSource Source);
}

namespace System.Collections.Generic
{
    public interface IAsyncEnumerable<T>
    {
        IAsyncEnumerator<T> GetAsyncEnumerator(CancellationToken cancellationToken = default);
    }

    public interface IAsyncEnumerator<T>
    {
        ValueTask<bool> MoveNextAsync();
        T Current { get; }
    }
}

[AlekseyTs]

@jcouv FYI

[AlekseyTs]

@henriquewr Please use more detailed PR title. Also, please include detailed description of the the change: what you are planning to change, how do you plan to change, why, why do you think this is a right change, etc.