Allow multiple calls to GetAsyncEnumerator

[jcouv]

The state machine for iterator methods is meant to be used as an enumerable which can produce multiple enumerators, but it also acts as the first enumerator that is returned.
The state machine for async methods is meant to be used only once. The state machine for async-iterator initially followed that design, but we're changing that.

With this PR, the setup of an async-iterator method becomes more like that of an iterator method than that of an async method.
Every time you call GetAsyncEnumerator(), you get a fresh enumerator (either by recycling one that is finished, or by instantiate and initializing a new one.
This implies that the state machine for async-iterator methods must keep a copy of the original parameters.

Since this design is very much like iterator methods, I have refactored some of the code from IteratorRewriter as helper methods and re-used them.

Fixes #30275

Before:

	[CompilerGenerated]
	private sealed class <M>d__0<T> : IAsyncEnumerable<T>, IAsyncEnumerator<T>, IAsyncDisposable, IValueTaskSource<bool>, IStrongBox<ManualResetValueTaskSourceLogic<bool>>, IAsyncStateMachine
	{
...
		[DebuggerHidden]
		ValueTask<bool> IAsyncEnumerator<T>.MoveNextAsync()
		{
			if (<>1__state == -2)
			{
				return default(ValueTask<bool>);
			}
			<>v__promiseOfValueOrEnd.Reset();
			<M>d__0<T> stateMachine = this;
			<>t__builder.Start(ref stateMachine);
			return new ValueTask<bool>(this, <>v__promiseOfValueOrEnd.Version);
		}

		[DebuggerHidden]
		IAsyncEnumerator<T> IAsyncEnumerable<T>.GetAsyncEnumerator()
		{
			return this;
		}
	}

	[AsyncStateMachine(typeof(<M>d__0<>))]
	[IteratorStateMachine(typeof(<M>d__0<>))]
	private static IAsyncEnumerable<T> M<T>(T value)
	{
		<M>d__0<T> <M>d__ = new <M>d__0<T>();
		<M>d__.value = value;
		<M>d__.<>t__builder = AsyncVoidMethodBuilder.Create();
		<M>d__.<>1__state = -1;
		<M>d__.<>v__promiseOfValueOrEnd = new ManualResetValueTaskSourceLogic<bool>(<M>d__);
		return <M>d__;
	}

After:

	[CompilerGenerated]
	private sealed class <M>d__0<T> : IAsyncEnumerable<T>, IAsyncEnumerator<T>, IAsyncDisposable, IValueTaskSource<bool>, IStrongBox<ManualResetValueTaskSourceLogic<bool>>, IAsyncStateMachine
	{
...
		[DebuggerHidden]
		public <M>d__0(int <>1__state)
		{
			this.<>1__state = <>1__state;
			<>l__initialThreadId = Environment.CurrentManagedThreadId;
			<>t__builder = AsyncVoidMethodBuilder.Create();
			<>v__promiseOfValueOrEnd = new ManualResetValueTaskSourceLogic<bool>(this);
		}

		[DebuggerHidden]
		IAsyncEnumerator<T> IAsyncEnumerable<T>.GetAsyncEnumerator()
		{
			<M>d__0<T> <M>d__;
			if (<>1__state == -2 && <>l__initialThreadId == Environment.CurrentManagedThreadId)
			{
				<>1__state = 0;
				<M>d__ = this;
			}
			else
			{
				<M>d__ = new <M>d__0<T>(0); // make a new enumerator instance if needed
			}
			<M>d__.value = <>3__value; // copy parameters
			return <M>d__;
		}
	}


	[AsyncStateMachine(typeof(<M>d__0<>))]
	[IteratorStateMachine(typeof(<M>d__0<>))]
	private static IAsyncEnumerable<T> M<T>(T value)
	{
		<M>d__0<T> <M>d__ = new <M>d__0<T>(-2);
		<M>d__.<>3__value = value; // save parameters
		return <M>d__;
	}

[cston]

} [](start = 7, length = 2)

Perhaps test overlapping enumerators with calls to each outside the lifetime of the other.

var enumerator1 = enumerable.GetAsyncEnumerator();
await enumerator1.MoveNext();
var enumerator2 = enumerable.GetAsyncEnumerator();
await enumerator2.MoveNext();
await enumerator1.DisposeAsync();
await enumerator2.MoveNext();
await enumerator2.DisposeAsync();

[jcouv]

Good idea. I'll add

[agocke]

Agreed, especially with an await in between.

[jcouv]

@dotnet/roslyn-compiler for a second review. Thanks

[jcouv]

@dotnet/roslyn-compiler for second review. Thanks

[agocke]

LGTM, once the new test is added.

[agocke]

As I mentioned offline, I don't really understand our use of thread id here.

The other thing to consider: if our goal is to reuse the iterators on subsequent calls, a common pattern is probably going to be,

getenumerator && foreach
await
getenumerator && foreach
await
...

During each of those awaits we could be shoved onto the thread pool and come back on a different thread, but in the same task. In those cases it seems like what we would want is not the thread ID, but an async local marking whether or not it's OK to come back.

[jcouv]

I added a note to workitems list to confirm the whole threadID design, to re-convince ourselves that it is sound.

[jcouv]

@agocke It turns out I had looked into the threadID question at some point already.
https://github.com/dotnet/corefx/issues/3481 discusses the trade-offs of this design for iterators and concluded to keep the threadID design. The same analysis applies to async-iterators.

[agocke]

Agreed, especially with an await in between.