System.Interactive.Async with C# 8.0 proposed interfaces

[bartdesmet]

This PR is not intended to be merged into the develop or master branch at this point. It's used as a playground to see the changes that'd be needed to System.Interactive.Async to support LINQ operators using the interface definitions proposed for C# 8.0 up to the 8/30.2017 C# language design meeting.

See dotnet/csharplang#866 for the PR containing these interface definitions, merged into Async Streams.

The initial iteration deals with all the interface changes, and gets us to a compiling solution, without deep consideration of breaking changes compared to the existing assembly. Tests have to be reviewed. We can use this PR to discuss pros and cons of the current interface design.

---

Update 10/5/18: Supports latest interface definitions as per https://github.com/dotnet/corefx/issues/32640

namespace System
{
    public interface IAsyncDisposable
    {
        ValueTask DisposeAsync();
    }
}

namespace System.Collections.Generic
{
    public interface IAsyncEnumerable<out T>
    {
        IAsyncEnumerator<T> GetAsyncEnumerator();
    }

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

---

Update 11/28/18: Supports latest interface definitions as per https://github.com/dotnet/csharplang/blob/master/meetings/2018/LDM-2018-11-28.md

namespace System
{
    public interface IAsyncDisposable
    {
        ValueTask DisposeAsync();
    }
}

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

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

[clairernovotny]

For some of the inner-loop async things, like in the aggregators, should it be ValueTask<T> instead of Task<T>?

[bartdesmet]

I've spent a few days to refresh "Interactive Extensions" (Ix) for IAsyncEnumerable<T> to be in sync with the latest LDM notes (https://github.com/dotnet/csharplang/blob/master/proposals/async-streams.md).

In a nutshell, this work enables "async LINQ to Objects" and has all of the Standard Query Operators available.

To download the NuGet packages, follow the link to the AppVeyor build, where you can download the latest NuGet packages from the "Artifacts" tab.

A summary of the work done:

• I've refactored System.Interactive.Async into multiple pieces:

  • System.Linq.Async contains the interfaces and the System.Linq.AsyncEnumerable class containing LINQ extension methods.
  • System.Linq.Async.Queryable contains IAsyncQueryable<T> and the (generated) System.Linq.AsyncQueryable class containing the expression equivalent of the above.
  • System.Interactive.Async contains System.Linq.AsyncEnumerableEx and contains non-standard LINQ extension methods, analogous to the (synchronous) ones found in the System.Interactive assembly.
  • System.Interactive.Async.Providers contains System.Linq.AsyncQueryableEx for the previous one. (The Providers nomenclature is a remnant of Ix naming).

• The most notable differences between the original "Ix-Async" implementation and the PR are:

  • Rename of IAsyncEnumerable<T>.GetEnumerator to IAsyncEnumerable<T>.GetAsyncEnumerable. This change was a trivial rename.
  • Change of IAsyncEnumerator<T> implementing IDisposable to implementing IAsyncDisposable (added to the assembly). This change was fairly trivial but the lack of using await is notable.
  • Rename and change of IAsyncEnumerator<T>.MoveNext(CancellationToken) to IAsyncEnumerator<T>.MoveNextAsync(). This change was the most tedious and involved removing fine-grained cancellation support from Ix-Async.

• Async overloads for Standard Query Operators:

  • Aggregates in "Ix-Async" already had a Task<T> return type and accepted an optional CancellationToken. The only difference now is that the CancellationToken is used in a shallow manner to break out of the aggregation loops.
  • Every operator with a Func<T1, ..., Tn, R> parameter now has an overload with a Func<T1, ..., Tn, Task<R>> overload.
  • Every operator with multiple delegate parameters has both all-sync and all-async overloads.

Open questions and topics for discussion:

• Lack of fine-grained cancellation support on MoveNextAsync remains to bring mixed feelings:

  • "Ix-Async" has always had such fine-grained cancellation support which was wired through the operators. Removing it is at the very least a regression for users of Ix-Async (including Entity Framework).
  • I do appreciate the complexity it brings for foreach await, async iterators, and query comprehensions.
    • Right now, "Ix-Async" hand rolls all of these without language support, so wiring CancellationTokens is not a big deal.
    • Also, "Ix-Async" had very few places where a Register call was made on such tokens; in most cases, the token simply was flowing through calls.
    • It wouldn't be a big deal - though tedious and bringing overload hell - for "Ix-Async" to have operators with delegate parameters that have the Func<T1, ..., Tn, CancellationToken, Task<R>> form.
  • An option would be for the query operators to be implemented using an (internal?) interface variant to keep the fine-grained cancellation support:
    • Without a good consumption and (to a lesser extent, e.g. for sources) production pattern it's unlikely such cancellation will be truly leveraged though.
    • This could also be a place to consider the TryGetNext interface approach to achieve performance gains within the query operators.
  • Having an IAsyncEnumerable<T> refer to a CancellationToken (e.g. in the closure of a lambda parameter passed to an operator) seems fundamentally unsound:
    • A "cold" sequence referring to a "hot" token undoes the "cold" nature of the sequence. A single cancellation will reach all consumers and the "cold" object will become unusable afterwards.
    • We can't prevent users from doing this even if cancellation tokens would "flow", similar to accessing shared mutable state within a query expression, but we may end up with coding anti-patterns:
      • E.g. IAsyncEnumerable<T> Get<T>(CancellationToken token) feels icky.
      • The question as to whether IAsyncEnumerator<T> will be a more commonly used type (compared to IAsyncEnumerable<T>) remains a pertinent one.
  • Various (non-aggregating) query operators can be non-interruptible when no CancellationToken can be passed in:
    • Example 1: Skip(n) for a large value of n is uninterruptible; the inner loop has no awareness of a token unless it flows through the query operators. Passing a CancellationToken to Skip in the IAsyncEnumerable<T> space doesn't feel right, as mentioned above.
    • Example 2: xs.Where(f) may evaluate the predicate on a lot of elements before yielding an element, allowing the consumer to break from the iteration. While the user could observe a cancellation token from the predicate, it suffers from the same issue as mentioned above.
    • Example 3: Range(0, int.MaxValue) will yield a lot of elements in a non-interruptible way. Again, passing CancellationToken feels wrong. This method acts as a "source", unlike the first example which is an operator.

• Use of Task<T> versus ValueTask<T> for query operator delegate parameter return types:

  • We don't add all possible sync/async variants for query operator overloads; just all-sync and all-async, so the use of Task.FromResult is expected to be common.
  • Language support is no longer an argument against such an approach (task-like return types in C# 7).
  • What are the performance trade-offs to consider here?
    • Bloating async state machine object size when using ValueTask<T>?
    • Path length for awaiting either of the two.
    • Considerations around the (likely) common case of users having a partially async requirement for using a query operator (e.g. SelectMany, GroupBy, GroupJoin, Join all have multiple delegates)
    • Reduction of System.Linq.Async assembly size? Right now, the logic of async iterators for all-sync and all-async variants of operators is duplicated, so all-sync doesn't use the all-sync one with Task.FromResult.

• Expression tree variants:

  • IAsyncQueryable<T> will construct expression trees in the "async space":
    • This has the predictable result of a homo-iconic representation compared to AsyncEnumerable:
      • AsAsyncQueryable is supported, and enumeration over it will rebind AsyncQueryable calls to the corresponding AsyncEnumerable calls.
    • Query providers that want to support IAsyncEnumerable<T> can rewrite expressions from AsyncQueryable to Queryable, but:
      • May have to strip out CancellationToken parameters, and shake off Task<T> return types in various lambdas.
      • May have to deal with a variety of Task-related members such as Task.FromResult.
  • It goes without saying that the lack of async and await in expression tree language support reduces the mileage of IAsyncQueryable<T>. We can look into reviving https://github.com/bartdesmet/ExpressionFutures/tree/master/CSharpExpressions.

• Home for auxiliary interfaces:

  • LINQ operators require the folllowing auxiliary interface async variants:
    • IAsyncGrouping<TKey, TElement>
    • IOrderedAsyncEnumerable<T>
    • IAsyncQueryable and IAsyncQueryable<T>
    • IOrderedAsyncQueryable and IOrderedAsyncQueryable<T>
    • IAsyncQueryProvider
  • Should these be added to .NET?

• Non-standard methods and query operators. Right now, the PR has the following non-standard methods that are open for debate:

  • ForEachAsync as a substitute for foreach await syntax, with predictable signatures. This could be a temporary addition until the language has foreach await.
  • Cast and OfType taking in an IAsyncEnumerable<object> rather than a non-generic interface:
    • Should there be a non-generic interface variant for IAsyncEnumerable? Likely not, though for IAsyncQueryable it seems we want one (query providers often want to cast without knowing the type parameter).
  • ToAsyncEnumerable conversions:
    • From IEnumerable<T> is quite straightforward.
    • From IObservable<T> needs design discussion on various buffering policies.
    • From Task<T> is debatable (could also first convert to IObservable<T> or reconsider having Task<T> implement IObservable<T>).
  • Conversions to other types:
    • To IEnumerable<T> incurs blocking.
    • To IObservable<T> is relatively straightfoward.
    • Others???
  • Append, Prepend, TakeLast, SkipLast, and ToHashSet were added to .NET Core, so likely no issue with these.
  • System.Interactive.Async adds non-standard operators that may be worth reviewing:
    • Async variants of existing Ix operators:
      • Min and Max with IComparer<T> (missing from BCL, may be considered for addition).
      • Distinct with a key selector (missing from BCL, may be considered for addition, possibly as DistinctBy).
      • Concat with an I[Async]Enumerable<IAsyncEnumerable<T>> can do a more performant job.
      • More, see code.
    • Operators that may make sense for IAsyncEnumerable<T>:
      • Merge for a concurrent merge of 2 or more sources.
      • Timeout where MoveNextAsync gets wrapped with a Task.WhenAny using Task.Delay could be useful.
      • Amb/WhenAny/Race to apply Task.WhenAny to 2 or more sources upon performing the first MoveNextAsync in order to elect and use a winner (and dispose the other).

• Some naming:

  • Should the Task<T>-returning aggregate methods have an Async suffix for the method names?

[akarnokd]

Close & defer until C# 8?

[clairernovotny]

This is the reference impl for C# 8.

[akarnokd]

This is a huge PR and even if we could review it, I don't see any indication the feedback could be addressed prior to merging this. Also I'm not following the C# 8 spec but is this based on the most recent design?

[akarnokd]

I suggest closing this PR for now. The code is still in the IxAsyncCSharp8 which will likely need extensive adjustments/rewrite eventually.

[danielcweber]

Good with me as long as the branch will survive. It's probably been abandoned anyway.

[bartdesmet]

I have some changes to this branch but no longer have permission to push.

[clairernovotny]

@bartdesmet Sorry, must have gotten lost in the move to this org, will fix now

[clairernovotny]

Should be fixed now, your'e an admin here.

[clairernovotny]

🎉 to the change on the get enumerator call

[bartdesmet]

Latest commit to experiment with CancellationToken passed to GetAsyncEnumerator per the ongoing discussion over at https://github.com/dotnet/corefx/issues/32640. By no means final; all three options:

• the original with CancellationToken on MoveNextAsync;
• the current C# 8.0 language proposal with no CancellationToken on the interfaces;
• the latest ongoing discussion with CancellationToken on GetAsyncEnumerator

are doable based on the experiments in this branch. Once the interfaces are finalized, we'll proceed accordingly. For discussions on the interface design, please join on https://github.com/dotnet/corefx/issues/32640.

[akarnokd]

How should we proceed with this branch? Start reviewing the unit tests? Post fixes/improvements to the tests/operators to this branch?

• Option 1 complicates the operators as each now should defensively assume the token is different and keep deregistering/registering to them. Plus it adds a lot of overhead per item.
• Option 2 requires certain operators requiring eager cancellation to provide awkward APIs as the worst case, each source needs its own specific token handed out and there could be arbitrary number of sources involved over time (example: Switch).
• Option 3 is the middle ground and doesn't complicate things much, but there seems to be trouble with any compiler/language support. If these interfaces were purely for library usages, this option would be the most economic of the three.

[akarnokd]

Quick review:

• A lot of underscores missing.
• Many coordinating operators are missing (Merge, Switch, TakeUntil)
• Amb/Zip more than two sources missing.
• Handling the crashes of awaits in Amb/Zip
• Timed operators are missing (Timer, Interval, Timeout, generally operators with time constraints)
• Others act seemingly sequentially (SelectMany, GroupBy)
• Call to await DisposeAsync().ConfigureAwait(false); before returning false in MoveNextAsync, implying eager dispose as well as duplicate call to DisposeAsync as any foreach async/using async/consumer should call it anyway.
• Not sure about having ConfigureAwait(false) thus continuations may run all over the place.
• ToObservable doesn't seem to be protected against reentrance/effectively synchronous IAsyncEnumerables.
• Many lambda creation and anonymous "generators".
• Locks.
• The intent behind AsyncIterator, its own state machine, its cloning.
• Some internal methods are defined async Task instead of async ValueTask (example: Union).
• No subject analogues?

[akarnokd]

Would it be possible if the Ix.Async related code be moved into its standalone solution?

[clairernovotny]

@bartdesmet

We need to revert the two commits that removed the reference assemblies and REFERENCE_ASSEMBLY defines.

The issue is that we need to expose different surface area for netstandard2.0, netcoreapp2.1, and netstandard2.1 because of TakeLast, etc. We need the impl to always be there in the lib or we'll create runtime errors for people. We just can't expose those methods.

ProduceReferenceAssembly isn't the hard part, it's the different surface area and creating the correct NuGet package, both of which were handled by the define and Extras.

[clairernovotny]

Please don't make code style changes in this branch. It'll create harder to review merges and should be a seperate PR for discussion and review.

[clairernovotny]

@bartdesmet I added a property BuildForCSharp8 that is in the Directory.build.props that defaults to true and sets the CSHARP8 symbol. If you want to build without it, just set /p:BuildForCSharp8=false

I also added the .NET Core 3.0.100-preview to the global.json so it'll use the latest preview SDK you have on your machine. LangVersion was already set to latest, so you should be able to use C# 8 features as they're available in the SDK.

[quinmars]

Is the big rename really necessary? In my eyes method names like LastAwaitWithCancellationAsync are plain ugly. And it's not needed, because there is not any ambiguity for predicates. For selectors I see the point, but even there you might get some funny results with the new names:

var result = seq.OrderBy(async x => x.GetNameAsync());

I doubt that there are many cases where you actually want the non-awaited variant. So wouldn't it be better to treat the exception special than to make it the default?

If one really wants a sequence of tasks he could then do something like that:

var list = await seq.RawSelect(s => new ValueTask(s)).ToListAsync();
// or
await Task.WhenAny(seq.RawSelect(async x => await x.GetNameAsync()).ToArrayAsync());

[bartdesmet]

I do fully agree with the sentiment on the ugliness of names and this is by no means final; we may totally go back to the original state once we figure out the scope of potential issues. For the preview we're about to release, we wanted to err on the safe side and have all existing LINQ operators be 1:1 identical across System.Linq and System.Linq.Async (modulo the enumerable type of course), and also avoid accidental awaiting in places that may be unexpected.

After this preview, we will evaluate the pros and cons of disambiguation by name. An initial gut feeling of running the whole API by some corefx/language folks was to disambiguate operators with async selectors, predicates, etc. using an Async suffix, as in SelectAsync. However, that gets really funny once you start taking aggregates into account, where you already have an Async suffix due to the async return type. We clearly don't want to have AsyncAsync :-), so for the time being I ended up settling for Await.

We should have the discussion at length after getting an initial preview out, but some factors to consider are the following:

• Do all operators with synchronous delegate parameters work identical between sync/async LINQ? E.g. xs.Select((x, i) => f(x, i)) doesn't work in a "flat" world, because i could be int or CancellationToken.
  • The whole business with deep wiring of cancellation (which is actually guarded by an #if) leads to funny results because it results in overloads with delegate parameters that have the same parameter count but different types.
  • It gets particulary bad if you have sequences of anonymous types, so you can't specificy generic arguments on the operator to disambiguate (we don't have var for partial type inference on generics or lambda parameters).
• Should awaiting of asynchronous operations be made explicit or not?
  • One side of the argument is that you're already in an asynchronous world with IAsyncEnumerable<T>, so it'd be fair for everything to be implicitly awaited.
  • A possible issue is that it could make sense to using operators like Select, SelectMany, etc. to generate a sequence of asynchronous operations to feed into Task.When*. Though an escape hatch can be to use AsTask() in the body of these, because we're using ValueTask<> return types. But there's been discussion on implicit conversions for task types, so that'd make things brittle.
  • Possible prior design points (however few) could be harvested from System.Threading.Tasks, where ContinueWith does not have a variant with an async return type (i.e. t.ContinueWith(t => t) produces a Task<Task<T>>), so there's no implicit flattening, while Task.Run does have overloads with async functions that do perform flattening of tasks.
• Any considerations around extending query expressions in C#/VB to support asynchronous operations in query clauses.
  • This hasn't yet been given much thought by the C# LDM, but we can try to put it on the agenda.
  • I have done some preliminary study on possible issues, especially around clauses that generate multiple lambda expressions where you can get combinatorial explosion of sync/async variants (e.g. Join and GroupJoin generate three lambdas to bind). This gets into the discussion of overloads, where we currently take a sane stance of "all sync" and "all async".
  • At the very least, even if it's decided to not do any work on query expressions to support asynchronous operations in query clauses, we should make sure that a "flat" world does not lead to binding issues.

Just drilling a tiny bit deeper into the LINQ query expression binding, consider the following query:

from x in xs
from y in ys
select e

which desugars into

xs.SelectMany(x => ys, (x, y) => e)

xs can already be an expression containing await, provided the enclosing method or local function is async. For ys or e to contain asynchronous operations, the collection selector and/or result selector need to be desugared as async lambdas, so there are 4 possible combinations:

xs.SelectMany(x => ys, (x, y) => e)
xs.SelectMany(async x => ys, (x, y) => e)
xs.SelectMany(x => ys, async (x, y) => e)
xs.SelectMany(async x => ys, async (x, y) => e)

I'll ignore the syntactic aspects of query expressions that containing await in query clauses, except for some thoughts:

• One could argue that no syntactic changes are required, and the use of await in a clause results in the synthesis of an async lamdba (thus, the explicit nature of async that exists in all other places to "ask for async permission" does not apply here), which may or may not compile depending on the target methods the expression gets lowered and bound to.
• One could argue for explicitness either at the whole query level (on async from at the top) or in individual clauses (but async where await f(x) seems unweildy, though syntactically not remote from async x => await f(x), and not every clause has a natural place to drop in an explicit async).

Either way, let's assume a query clause can be asynchronous, and we have to lower and bind it by generating async lambdas. For SelectMany, there are 4 variants. For other clauses, the combinations look like this:

• Where, Select, OrderBy[Descending] and ThenBy[Descending] have 2.
• SelectMany and GroupBy have 4.
• Join and GroupJoin have 8.

One extreme is to support all overloads but it quickly leads to insanity. Although not all of them need to be implemented from scratch by a library; a valid implementation could just have "all sync" and "all async" master overloads, and the "one or more async" overloads could delegate into the "all sync" one by wrapping the result of the sync delegates into ValueTask<T> objects (which is a cheap struct), e.g.

static IAsyncEnumerable<R> SelectMany<T, C, R>(this IAsyncEnumerable<T> source, Func<T, IAsyncEnumerable<C>> collectionSelector, Func<T, C, R> resultSelector) => ...;

static IAsyncEnumerable<R> SelectMany<T, C, R>(this IAsyncEnumerable<T> source, Func<T, ValueTask<IAsyncEnumerable<C>>> collectionSelector, Func<T, C, ValueTask<R>> resultSelector) => ...;

static IAsyncEnumerable<R> SelectMany<T, C, R>(this IAsyncEnumerable<T> source, Func<T, ValueTask<IAsyncEnumerable<C>>> collectionSelector, Func<T, C, R> resultSelector) =>
    SelectMany<T, C, R>(source, collectionSelector, (x, y) => new ValueTask<R>(resultSelector(x, y)));

static IAsyncEnumerable<R> SelectMany<T, C, R>(this IAsyncEnumerable<T> source, Func<T, IAsyncEnumerable<C>> collectionSelector, Func<T, C, ValueTask<R>> resultSelector) =>
    SelectMany<T, C, R>(source, x => new ValueTask<IAsyncEnumerable<C>>(collectionSelector(x)), resultSelector);

This would be the easiest story for the language to lower and bind. Same methods names as the original ones, just decide to lower by using zero or more async lambdas, and see what binding looks like. Thus, the library is responsible to provide all combinations, which intersects with the space of possible overload resolution conflicts. It further bleeds into a discussion around:

• Should all operators (rather than just query expression lowering targets, which are C#/VB specific) have the full matrix of combinations, for consistency?
• Overloads with CancellationToken make the situation twice as bad. All or nothing or the whole combinatorial space?

Other options of async in query expressions include:

• "1 or more async clause becomes all-in async", thus be happy to lower to async x => y even if y does not contain any await expression (and suppress the traditional warning of "will run synchronously because no await expressions occur"). That is, the language binding brings sanity, and libraries do "all or nothing" async.
• Bind using different names as soon as some "async" occurs, either because imposing various overloads for existing query operator methods leads to ambiguity (i.e. this whole discussion), or some other reason we haven't figured out yet.

Either way, I think the whole overload space with async delegates and/or cancelable async delegates warrants input from the .NET corefx team, the C# LDM team, library users (such as Entity Framework), and end users. It may also be the source of API design guidelines for methods with delegate parameters that want to support "deep" asynchrony with or without cancellation, similar to the "Task-based Asynchronous Pattern (TAP)".

For now, we'll freeze the API surface for a first preview drop of System.Linq.Async and System.Interactive.Async, and put this discussion on the top of the list for future previews and the eventual "RTM" release of these libraries.

[bartdesmet]

In order to evaluate both API options to support async and "deep cancellation", I've centralized all of these overloads into a single place for corefx and LDM folks to review. See AsyncEnumerable.AsyncOverloads.cs which has an #if SUPPORT_FLAT_ASYNC_API option to switch between the flat world and the Await[WithCancellation] world. Once a decision has been made either way, we can do away with the indirection using some trivial search-and-replaces.

As discussed above, the preview will ship with SUPPORT_FLAT_ASYNC_API turned off, so we start with fine-grained overloads and can move to a "flat" coarse-grained world later if all possible roadblocks have been inspected and cleared.

Anyone can build the "flat" world to play around with it as well to identify possible issues, but note that the test code current fails to build with that option turned on, revealing some of the places of concern where lambda parameters can't be inferred. If I get some time in the next few weeks, I'll continue to clean up the test code and add a corpus of "asyncified" queries, originally expressed against IEnumerable<T>, that should work as-is in the IAsyncEnumerable<T> world. At that point, looking at the list of compile errors when SUPPORT_FLAT_ASYNC_API is turned on will give us a data point of the possible surprises users can run into.

[slang25]

Amazing! 🎉 I can't wait to have a play about with this 😃

[clairernovotny]

The CI feed has it now, just awaiting a few sanity checks before publishing to nuget.

[bartdesmet]

I've also created an early prototype of async query expressions in C# over at https://github.com/bartdesmet/roslyn/blob/AsyncLinq. This enables await in query expressions and binds correctly against the System.Linq.Async library provided here.

Some docs can be found here, also summarizing some of the discussion we've been having here.

[danielcweber]

@bartdesmet Upon playing a bit with async iterators in conjunction with Rx, I found that the Ix-Rx-bridging operator ToObservable does not play nicely with how Roslyn lowers async iterators. Specifically, DisposeAsync may not be called in-flight and will throw.

Whether the NotSupportedException shouldn't rather be an InvalidOperationException is not the subject of this review, however, it is a breaking change to how the "classic" Ix.Async implementation would handle in-flight disposal (it would do a best-effort approach to cancel MoveNext).

[akarnokd]

I had to trampoline the async Dispose with the async MoveNext in my implementation. It was painful for my performance-sensitive eyes.

[danielcweber]

But Dispose doesn't cancel in-flight MoveNextAsync, does it?

[quinmars]

I think on Dispose, the cancellation token source should be canceled. That is probably why there is a cancellation token source at all.

[danielcweber]

I'm afraid the compiler that implements the async iterator doesn't put a CancellationTokenSource into it...if I didn't miss anything.

[quinmars]

That's right. And it's also right that the AsyncDispose at that position is a contract violation.

[danielcweber]

It is, and also the fact that it is thrown synchronously caught me at one point when playing around.

[danielcweber]

Clarification: Roslyn, at least on the current master (porbably not in any stable release of VS) does indeed create a CancellationTokenSource if the signature for the async iterator contains a CancellationToken parameter. If created in every case, it could help fix the Dispose-issue.

@jcouv Would you be so kind to weigh into this conversation?

[jcouv]

DisposeAsync on a generate async-iterator does two things: it executes any finally blocks from the current yield statement, and it disposes the CancellationTokenSource (if any).

From the design notes:

The caller of an async-iterator method should only call DisposeAsync() when the method completed or was suspended by a yield return.

LDM did briefly discuss allowing DisposeAsync() to be called any time, as a way to cancel an iteration, but this would require some synchronization which we felt was too much overhead.

[danielcweber]

I guess it's then up to System.Linq.Async / System.Interactive.Async to work around that. Otherwise virtually any Rx-operator-chain may throw on unsubscription if there's a conversion from an async iterator to an IObservable somewhere in between.