[ManagedHandler] Improve cancellation support when sending/receiving data

[stephentoub]

The ManagedHandler_CancellationTest tests are disabled because sockets don't support canceling individual operations. We might be able to address this once https://github.com/dotnet/corefx/issues/22608 is implemented and the associated cancellation support is enabled. We could also employ a variety of workarounds, including ending the whole request on cancellation by disposing of the connection, wrapping the operations in ones that can be canceled even though the underlying operation may continue running, etc.

[WinCPP]

@stephentoub Before submitting any fix, I wanted to witness the error, so uncommented the associated test ManagedHandler_CancellationTest at this link where I found reference for this issue dotnet/corefx#23141. I also commented of the reference to ActiveIssue #21400 at this line.

Still, I am not able to reproduce the issue by giving command msbuild /t:RebuildAndTest in the folder ./src/System.Net.Http/tests/FunctionalTests where the test resides. I even tried with parameter /p;framework=netcoreapp to msbuild command.

Am I missing something? Appreciate your inputs.

[stephentoub]

@WinCPP, the test is marked [OuterLoop], so you need to add /p:Outerloop=true to the msbuild command.

[WinCPP]

Hi @stephentoub , I think I have some solution. The method GetAsync_ResponseContentRead_CancelUsingTimeoutOrToken_TaskCanceledQuickly (link) annotated with attribute ActiveIssue #21400 no more fails. However I noticed that the test case 'ReadAsStreamAsync_ReadAsync_Cancel_TaskCanceledQuickly' (link) in the same file fails i.e. blocks, on my PC and, without commenting it out, I am not able to test the GetAsync... method.

I am not sure if ReadAsStreamAsync_ReadAsync_Cancel_TaskCanceledQuickly should block; it doesn't have any ActiveIssue annotation for netcoreapp framework. Please let me know if this is expected behavior on PC.

Also should I raise a PR for whatever code changes I have done...? Appreciate your inputs even if I am seeing the above mentioned issues?

[stephentoub]

However I noticed that the test case 'ReadAsStreamAsync_ReadAsync_Cancel_TaskCanceledQuickly' (link) in the same file fails i.e. blocks

That would seem to be due to your changes then. It's currently passing on .NET Core.

[stephentoub]

Also should I raise a PR for whatever code changes I have done...?

Sure, but I'm not sure how you'll be successful at this presently, given that the underlying sockets don't yet have actual support for cancellation enabled. What is the solution you're attempting?

[WinCPP]

Hi @stephentoub,

The solution that I am attempting is based on one of the ways that you mentioned in the opening comment for this issue, quoted below,

wrapping the operations in ones that can be canceled even though the underlying operation may continue running, etc.

I came across this blog of yours on which the solution is based. I understand that it may not be clean solution but it kind of adheres to one of the suggested solutions (for now until sockets get support for cancellation)...

I am fine tuning the code and also checking why ReadAsStreamAsync_ReadAsync_Cancel_TaskCanceledQuickly hangs... else will publish a PR for whatever I have coded in a day or two...

[WinCPP]

Hi @stephentoub I could make some progress on this. As for the hang in ReadAsStreamAsync_ReadAsync_Cancel_TaskCanceledQuickly, I found that it needs separate fix in 'ContentLengthReadStream' as after the response has been obtained, since ContentLengthReadStream doesn't handle cancellation via token and hence the hang. I am working on this. Also instead of allowing the underlying operation to continue, I am trying to find a way to close the httpconnection or the underlying stream (NetworkStream) so that there is an IOException or ObjectDisposedException, either of them triggered because the connection and / or the stream was disposed.

[WinCPP]

Hi @stephentoub thanks for the inputs in this comment. I will try working on it for dotnet/corefx#24430 and then make use of it here...

[geoffkizer]

@stephentoub per our email discussion, sounds like you have a solution for this...

[stephentoub]

We can do something similar to what WinCPP was working on. I'm working through some of the more intricate details. It's far from ideal, but I think it'll be shippable.