improve quic test stability for cancellation

[wfurt]

I think the CloseAsync was there for historical reasons to properly close.
This should no longer be needed as well as it creates problem for tests with cancellation or somewhat destructive behavir.
Depending on timing and situation this may fail with "Transport close" or "User aborted" message event if both test function succeeded.
if we want to keep something in place I think we should replace this with Abort/Reset. (but relying on Dispose looks good to me)
I did few hundreds runs and I did not see any issue.

[ghost]

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

---

I think the CloseAsync was there for historical reasons to properly close.
This should no longer be needed as well as it creates problem for tests with cancellation or somewhat destructive behavir.
Depending on timing and situation this may fail with "Transport close" or "User aborted" message event if both test function succeeded.
if we want to keep something in place I think we should replace this with Abort/Reset. (but relying on Dispose looks good to me)
I did few hundreds runs and I did not see any issue.

Author:	wfurt
Assignees:	wfurt
Labels:	area-System.Net.Quic
Milestone:	-

[danmoseley]

@MihaZupan this test hung on net6.0-Linux-Debug-x64-mono_interpreter_release-Debian.9.Amd64.Open

   System.Net.NameResolution.Functional.Tests: [Long Running Test] 'System.Net.NameResolution.Tests.TelemetryTest.ResolutionsWaitingOnQueue_ResolutionStartCalledBeforeEnqueued', Elapsed: 00:14:11
Killed
['System.Net.NameResolution.Functional.Tests' END OF WORK ITEM LOG: Command timed out, and was killed]

Related to your #56208 ?

[MihaZupan]

Yes, the test was added in that PR. I will take a look

[CarnaViire]

@wfurt could you please describe a situation that this is fixing? I still don't understand how both of the functions can return, (which should mean that all streams were finished and disposed) but the connections are still not save to call Close. Isn't that a symptom of client/server functions badly written e.g. leaking streams?
As for relying on dispose - Disposing connection would also call "Close" ("shutdown" in msquic language) but with more dangerous SILENT parameter, which IMO is even worse - AFAIK it will not notify peer of connection being closed, so it may just hide bad behavior, or peer might even end up hanging forever

[wfurt]

Here is example of failed run of ReadOutstanding_ReadAborted_Throws.

Test function calls serverStream.AbortRead(ExpectedErrorCode);

     System.Net.Quic.QuicOperationAbortedException : Operation aborted.
      Stack Trace:
        /home/furt/github/wfurt-runtime/src/libraries/System.Net.Quic/src/System/Net/Quic/Implementations/MsQuic/MsQuicStream.cs(1368,0): at System.Net.Quic.Implementations.MsQuic.MsQuicStream.HandleEventConnectionClose(State state)
        /home/furt/github/wfurt-runtime/src/libraries/System.Net.Quic/src/System/Net/Quic/Implementations/MsQuic/MsQuicStream.cs(952,0): at System.Net.Quic.Implementations.MsQuic.MsQuicStream.HandleEventShutdownComplete(State state, StreamEvent& evt)
        /home/furt/github/wfurt-runtime/src/libraries/System.Net.Quic/src/System/Net/Quic/Implementations/MsQuic/MsQuicStream.cs(808,0): at System.Net.Quic.Implementations.MsQuic.MsQuicStream.HandleEvent(State state, StreamEvent& evt)
        /home/furt/github/wfurt-runtime/src/libraries/System.Net.Quic/src/System/Net/Quic/Implementations/MsQuic/MsQuicStream.cs(770,0): at System.Net.Quic.Implementations.MsQuic.MsQuicStream.NativeCallbackHandler(IntPtr stream, IntPtr context, StreamEvent& streamEvent)
        --- End of stack trace from previous location ---
        /home/furt/github/wfurt-runtime/src/libraries/System.Net.Quic/src/System/Net/Quic/Implementations/MsQuic/MsQuicStream.cs(600,0): at System.Net.Quic.Implementations.MsQuic.MsQuicStream.ShutdownCompleted(CancellationToken cancellationToken)
        /home/furt/github/wfurt-runtime/src/libraries/System.Net.Quic/tests/FunctionalTests/QuicTestBase.cs(246,0): at System.Net.Quic.Tests.QuicTestBase`1.<>c__DisplayClass33_0.<<RunStreamClientServer>b__1>d.MoveNext()
        --- End of stack trace from previous location ---
        /home/furt/github/wfurt-runtime/src/libraries/System.Net.Quic/tests/FunctionalTests/QuicTestBase.cs(246,0): at System.Net.Quic.Tests.QuicTestBase`1.<>c__DisplayClass33_0.<<RunStreamClientServer>b__1>d.MoveNext()
        --- End of stack trace from previous location ---
        /home/furt/github/wfurt-runtime/src/libraries/System.Net.Quic/tests/FunctionalTests/QuicTestBase.cs(164,0): at System.Net.Quic.Tests.QuicTestBase`1.<>c__DisplayClass32_1.<<RunClientServer>b__0>d.MoveNext()
        --- End of stack trace from previous location ---

We got Connection close and that will break the ShutdownCompleted. It is likely this is specific to cases when test does something abortive. I understand the concern about silent failures. However, if the test care, it should probably verify what ever behavior in the callback-functions.

Perhaps different fix would be to another parameter to simply cleanup as fast as e can in abortive way, ignoring errors, making sure we do not crash.

Or alternatively avoid RunClientServer for dispose, cancellation or abort test cases.

Any thoughts on this @ManickaP @geoffkizer ???

[ManickaP]

@wfurt: Should this be closed in favor of merged #56839?

[wfurt]

I will wait. And see if we end up using different test pattern for the cases when the test cause damage to the provided objects.