HTTP/3: Cancellation causing ObjectDisposedException #56683
Comments
|
Tagging subscribers to this area: @dotnet/ncl Issue DetailsTest: ServerStreaming_CancellationOnClientWhileMoveNext_CancellationSentToServer I think this problem can be reproduced by: var response = await client.SendAsync(request);
var responseStream = await response.Context.ReadAsStreamAsync();
var readTask = responseStream.ReadAsync(new byte[1024);
response.Dispose();
var count = await readTask; // ErrorLogs:
|
dotnet-issue-labeler
bot
added
the
untriaged
|
I would say it's not cancellation per se, but using cancellation token together with Dispose - a race happens, so if disposal is faster, then cancellation token callback aborting the read will deal with stream already in disposed state. I guess we might want to swallow the exception here but I still wonder if it's the right thing to do. I mean, if Dispose is called together with something, ODE could be expected? Or am I wrong? I would guess there still might be a race between |
|
What would you expect @JamesNK ? It feels like |
|
I expect |
|
Dispose is a way to cancel. That is very much true. There is no similar race in HTTP/2 currently only because Cancellation token is not a way to cancel there and we discussed previously that it's a bug. By Cancellation token I mean the one passed to Content's ReadAsync. Meaning, it is also a response that has returned headers. Remembering gRPC code, maybe you may not pass full gRPC call's CT to ReadAsync (and only pass user provided one) -- as you call Dispose on it's triggering, so reading would still be cancelled by full gRPC call's CT, but there would be no race discussed here? Or do I miss something? |
|
Ok. I thought when you discussed a cancellation token you were referring to the one passed to Yes, gRPC will trigger both methods of cancellation together: |
|
@JamesNK is there something left for us to fix? If I get it right, this should be fixed in Grpc by not using the call's CT in @CarnaViire I remember we had a discussion saying that the problem is not the H/3 behavior but H/2 behavior which allows this to work. Do I remember it correctly or was it something else? If yes, is there any issue filed from that discussion? |
gRPC isn't doing anything wrong. It's triggering a cancellation token - a perfectly normal thing to do - then calling The problem is HttpClient turns these normal actions into a race that can explode. The race should be fixed in HttpClient. |
|
Just to make sure I understand this properly: There's a race between cancelling the ReadAsync on the response stream and disposing the response stream, as to which one causes the request to abort. Correct? Which case is it where you get the ODE? If Dispose happens first, it should cancel the read and the read should throw something like IOException("Request aborted"). Is that not what is happening today? |
|
Triage: we might get rid of the disposed check in abort methods on the stream. I'll talk with @CarnaViire about the exact place. Also she knows which test covers this, albeit not every time it runs. I might squash this with my work on #56129, which is related. |
ghost
added
the
in-pr
Allow calling Abort(Read|Write) after disposing. The only safe option to do so is to make them no-op in that case. Fixes #56683
ghost
removed
the
in-pr
ghost
locked as resolved and limited conversation to collaborators
Test: ServerStreaming_CancellationOnClientWhileMoveNext_CancellationSentToServer
Code: https://github.com/JamesNK/grpc-dotnet/tree/jamesnk/http3
I think this problem can be reproduced by:
Or it might be a CancellationToken passed to ReadAsync being canceled while waiting for content from the server. I think both are triggered when gRPC cancels a request.
Otherwise, debug using the gRPC test. It consistently throws this error.
Logs:
The text was updated successfully, but these errors were encountered: