Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

When clients dispose an IAsyncEnumerator, the AsyncEnumerableGrainExtension can throw a NotSupportedException #9185

Closed
Tragetaschen opened this issue Oct 16, 2024 · 5 comments · Fixed by #9186
Assignees

Comments

@Tragetaschen
Copy link

I've seen in my logs exceptions similar to this

[12:47:50 WRN] Orleans.Runtime.AsyncEnumerableRequest Failed to dispose async enumerator.
System.NotSupportedException: Specified method is not supported.
   at <my-iasyncenumerable-grain-method-using-the-c#-state-machine>+System.IAsyncDisposable.DisposeAsync()
   at Orleans.Runtime.AsyncEnumerableGrainExtension.DisposeAsync(Guid requestId) in /_/src/Orleans.Core/Runtime/AsyncEnumerableGrainExtension.cs:line 54
   at OrleansCodeGen.Orleans.Runtime.Invokable_IAsyncEnumerableGrainExtension_GrainReference_Ext_3C6D7209.InvokeInner() in /_/src/Orleans.Core.Abstractions/Orleans.CodeGenerator/Orleans.CodeGenerator.OrleansSerializationSourceGenerator/Orleans.Core.Abstractions.orleans.g.cs:line 241
   at Orleans.Runtime.Request.Invoke() in /_/src/Orleans.Core.Abstractions/Runtime/GrainReference.cs:line 592
--- End of stack trace from previous location ---
   at Orleans.Runtime.GrainMethodInvoker.Invoke() in /_/src/Orleans.Core/Core/GrainMethodInvoker.cs:line 132
   at OrleansDashboard.Implementation.GrainProfilerFilter.Invoke(IIncomingGrainCallContext context)
   at Orleans.Runtime.GrainMethodInvoker.Invoke() in /_/src/Orleans.Core/Core/GrainMethodInvoker.cs:line 94
   at Orleans.Runtime.InsideRuntimeClient.Invoke(IGrainContext target, Message message) in /_/src/Orleans.Runtime/Core/InsideRuntimeClient.cs:line 267
   at Orleans.Serialization.Invocation.ResponseCompletionSource.System.Threading.Tasks.Sources.IValueTaskSource.GetResult(Int16 token) in /_/src/Orleans.Serialization/Invocation/ResponseCompletionSource.cs:line 98
   at Orleans.Runtime.AsyncEnumeratorProxy`1.DisposeAsync() in /_/src/Orleans.Core.Abstractions/Runtime/AsyncEnumerableRequest.cs:line 210

As far as I can tell, the AsyncEnumerableGrainExtension has a bug, where the enumerator's DisposeAsync method is unconditionally called while there still might be an incomplete enumerator.MoveNextAsync task "running".

if (_enumerators.Remove(requestId, out var enumerator) && enumerator.Enumerator is { } value)
{
return value.DisposeAsync();
}

The C# compiler's state machine for an IAsyncEnumerable contains an explicit check for this condition and throws a NotSupportedException. It's a bit cumbersome, but can be seen here

@ReubenBond ReubenBond self-assigned this Oct 16, 2024
@ReubenBond
Copy link
Member

Thanks for reporting, @Tragetaschen. I can reproduce the issue here and am working on a fix

@ReubenBond
Copy link
Member

@Tragetaschen I've opened #9186 to address this

@Tragetaschen
Copy link
Author

I thought about this as fix before as well, but discarded it:
The grain method is not able to accept a [EnumeratorCancellation] CancellationToken cancellationToken. This in turn means it's neither stored/incorporated in the IAsyncEnumerable/IAsyncEnumerator state machine nor is it available in the grain method to cancel its own work. This essentially means the await task.SuppressThrowing() only completes when it generates another value, which it might not…

Or is there already support for native CancellationTokens on grain interfaces? 😁

@Tragetaschen
Copy link
Author

It is possible, though, to accept a CancellationToken without support on the Grain interface:

public IAsyncEnumerable<int> GrainInterfaceMethod() {
    return GrainInterfaceMethodImpl(default);
    async IAsyncEnumerable<int> GrainInterfaceMethodImpl([EnumeratorCancellation] CancellationToken cancellationToken) {
        yield return 420;
        await Task.Delay(100, cancellationToken);
        yield return 69;
    }
}

The default token is then combined with the one from the GetAsyncEnumerator call, but that "trick" isn't really obvious.

@ReubenBond
Copy link
Member

ReubenBond commented Oct 17, 2024

Hopefully, we can add native CancellationToken support for grain calls soon. There is a PR open that I need to review more and think about more (propagating cancellation down a call chain is a subtle topic)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants