dotnet / roslyn

The Roslyn .NET compiler provides C# and Visual Basic languages with rich code analysis APIs.
https://docs.microsoft.com/dotnet/csharp/roslyn-sdk/
MIT License
19.03k stars 4.03k forks source link

`await foreach` utilizing struct enumerator never ends #72820

Closed AlekseyTs closed 3 weeks ago

AlekseyTs commented 7 months ago
using System.Threading;
using System.Threading.Tasks;

struct S1
{
    public S2 GetAsyncEnumerator(CancellationToken token = default)
    {
        return new S2();
    }
}

struct S2
{
    bool stop;

    public ValueTask DisposeAsync()
    { 
        System.Console.Write("D");
        return ValueTask.CompletedTask;
    }

    public int Current => 123;
    public async ValueTask<bool> MoveNextAsync()
    {
        if (!stop)
        {
            stop = true;
            return true;
        }

        return false;
    }
}

class C
{
    static async Task Main()
    {
        await Test(new S1());
    }

    static async Task Test(S1 s1)
    {
        await foreach (var i in s1)
        {
            System.Console.Write(i);
        }
    }
}

Expected output: "123D"

Observed: an infinite loop

If S2 is changed to a class, the code behaves as expected. It is quite possible that generated code makes unexpected copies of the struct.

A generic case also exhibits the same problem:

 #nullable disable

using System;
using System.Threading;
using System.Threading.Tasks;

interface ICustomEnumerator
{
    public int Current {get;}

    public ValueTask<bool> MoveNextAsync();
}

interface IGetEnumerator<TEnumerator> where TEnumerator : ICustomEnumerator
{
    TEnumerator GetAsyncEnumerator(CancellationToken token = default);
}

struct S1 : IGetEnumerator<S2>
{
    public S2 GetAsyncEnumerator(CancellationToken token = default)
    {
        return new S2();
    }
}

struct S2 : ICustomEnumerator, IAsyncDisposable
{
    bool stop;

    public ValueTask DisposeAsync()
    { 
        System.Console.Write("D");
        return ValueTask.CompletedTask;
    }

    public int Current => 123;
    public async ValueTask<bool> MoveNextAsync()
    {
        if (!stop)
        {
            stop = true;
            return true;
        }

        return false;
    }
}

class C
{
    static async Task Main()
    {
        await Test<S1, S2>();
    }

    static async Task Test<TEnumerable, TEnumerator>()
        where TEnumerable : IGetEnumerator<TEnumerator>
        where TEnumerator : ICustomEnumerator, IAsyncDisposable
    {
        await foreach (var i in default(TEnumerable))
        {
            System.Console.Write(i);
        }
    }
}
AlekseyTs commented 7 months ago

CC @jcouv

jcouv commented 3 weeks ago

The problem doesn't seem to be specific to await foreach. The following code yields "TrueTrue" (which indicates that the change to stop was lost). I'm still investigating why this is happening and I'm not sure whether it's expected with struct and async.

using System;
using System.Threading.Tasks;

S2 s = new S2();
var b = await s.MoveNextAsync();
Console.Write(b);
var b2 = await s.MoveNextAsync();
Console.Write(b2);

struct S2
{
    bool stop;

    public async ValueTask<bool> MoveNextAsync()
    {
        await Task.Yield();
        if (!stop)
        {
            stop = true;
            return true;
        }

        return false;
    }
}
jcouv commented 3 weeks ago

Traced it down and I think this is expected. When we do await s.MoveNextAsync() we're starting a state machine with a copy of the struct, so any side-effects are lost after that. Closing as by-design

AlekseyTs commented 3 weeks ago

@MadsTorgersen Do you agree with the resolution? If so, it might be worth mentioning in the spec.

AlekseyTs commented 3 weeks ago

It looks like when MoveNextAsync is not implemented as an async method, the code behaves as expected. So, I agree that this is a problem with enumerator's implementation, not with await foreach code gen.

using System.Threading;
using System.Threading.Tasks;

struct S1
{
    public S2 GetAsyncEnumerator(CancellationToken token = default)
    {
        return new S2();
    }
}

struct S2
{
    bool stop;

    public ValueTask DisposeAsync()
    { 
        System.Console.Write("D");
        return ValueTask.CompletedTask;
    }

    public int Current => 123;
    public ValueTask<bool> MoveNextAsync()
    {
        if (!stop)
        {
            stop = true;
            return ValueTask.FromResult(true);
        }

        return ValueTask.FromResult(false);
    }
}

class C
{
    static async Task Main()
    {
        await Test(new S1());
    }

    static async Task Test(S1 s1)
    {
        await foreach (var i in s1)
        {
            System.Console.Write(i);
        }
    }
}