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

Invocation cancellation doesn't cancel the payload reading of payload writing #1610

Closed
bentoi opened this issue Aug 12, 2022 · 2 comments · Fixed by #1640
Closed

Invocation cancellation doesn't cancel the payload reading of payload writing #1610

bentoi opened this issue Aug 12, 2022 · 2 comments · Fixed by #1640
Assignees
Labels
bug Something isn't working

Comments

@bentoi
Copy link
Contributor

bentoi commented Aug 12, 2022

The following hangs right now:

var hangPayload = new HangPipeReader(); // a pipe reader whose ReadAsync hangs until canceled
var timeout = new CancellationTokenSource(100);
var request = new OutgoingRequest(new ServiceAddress(Protocol.IceRpc))
    {
         Payload = handPayload;
    };

connection.InvokeAsync(hangPayload, timeout.Token);

The problem is that we no longer create a linked token source in the invoke code to send the payload (we just abort the stream if the application cancellation token is canceled...). It's related to dotnet/runtime#69675 (comment)

We need to add tests to ensure that invocation and dispatch cancellation works even when the payload reader/writer hang.

@bentoi bentoi added the bug Something isn't working label Aug 12, 2022
@bernardnormier
Copy link
Member

I don't understand why we're not simply creating a linked token in InvokeAsyncCore and then pass this token to SendPayloadAsync.

@bentoi
Copy link
Contributor Author

bentoi commented Aug 16, 2022

It's in anticipation of the weird Quic implementation where the cancellation of stream.ReadAsync triggers under the hood the sending of the STOP_SENDING frame with the DefaultStreamErrorCode error code. See dotnet/runtime#69675 (comment) and the previous comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants