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

[HTTP/3] ReservedFrameType_Throws test fails #53090

Closed
ManickaP opened this issue May 21, 2021 · 14 comments · Fixed by microsoft/msquic#2401 or #67651
Closed

[HTTP/3] ReservedFrameType_Throws test fails #53090

ManickaP opened this issue May 21, 2021 · 14 comments · Fixed by microsoft/msquic#2401 or #67651
Assignees
Labels
area-System.Net.Http disabled-test The test is disabled in source code against the issue
Milestone

Comments

@ManickaP
Copy link
Member

System.Net.Http.Functional.Tests.SocketsHttpHandlerTest_Http3_MsQuic.ReservedFrameType_Throws

Intermittently fails with:

Assert.Throws() Failure\nExpected: typeof(System.Net.Quic.QuicConnectionAbortedException)\nActual:   typeof(System.Exception): unable to read request headers; unexpected end of stream.\n---- System.Exception : unable to read request headers; unexpected end of stream.
   at System.Net.Test.Common.Http3LoopbackStream.ReadRequestDataAsync(Boolean readBody) in /home/manicka/Repositories/runtime.2/src/libraries/Common/tests/System/Net/Http/Http3LoopbackStream.cs:line 179
   at System.Net.Test.Common.Http3LoopbackStream.HandleRequestAsync(HttpStatusCode statusCode, IList`1 headers, String content) in /home/manicka/Repositories/runtime.2/src/libraries/Common/tests/System/Net/Http/Http3LoopbackStream.cs:line 48
   at System.Net.Http.Functional.Tests.HttpClientHandlerTest_Http3.<>c__DisplayClass5_1.<<ReservedFrameType_Throws>b__2>d.MoveNext() in /home/manicka/Repositories/runtime.2/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http3.cs:line 136
--- End of stack trace from previous location ---
----- Inner Stack Trace -----
   at System.Net.Test.Common.Http3LoopbackStream.ReadRequestDataAsync(Boolean readBody) in /home/manicka/Repositories/runtime.2/src/libraries/Common/tests/System/Net/Http/Http3LoopbackStream.cs:line 179
   at System.Net.Test.Common.Http3LoopbackStream.HandleRequestAsync(HttpStatusCode statusCode, IList`1 headers, String content) in /home/manicka/Repositories/runtime.2/src/libraries/Common/tests/System/Net/Http/Http3LoopbackStream.cs:line 48
   at System.Net.Http.Functional.Tests.HttpClientHandlerTest_Http3.<>c__DisplayClass5_1.<<ReservedFrameType_Throws>b__2>d.MoveNext() in /home/manicka/Repositories/runtime.2/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http3.cs:line 136
--- End of stack trace from previous location ---
@ManickaP ManickaP added area-System.Net.Http disabled-test The test is disabled in source code against the issue labels May 21, 2021
@ManickaP ManickaP added this to the 6.0.0 milestone May 21, 2021
@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label May 21, 2021
@ghost
Copy link

ghost commented May 21, 2021

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

Issue Details

System.Net.Http.Functional.Tests.SocketsHttpHandlerTest_Http3_MsQuic.ReservedFrameType_Throws

Intermittently fails with:

Assert.Throws() Failure\nExpected: typeof(System.Net.Quic.QuicConnectionAbortedException)\nActual:   typeof(System.Exception): unable to read request headers; unexpected end of stream.\n---- System.Exception : unable to read request headers; unexpected end of stream.
   at System.Net.Test.Common.Http3LoopbackStream.ReadRequestDataAsync(Boolean readBody) in /home/manicka/Repositories/runtime.2/src/libraries/Common/tests/System/Net/Http/Http3LoopbackStream.cs:line 179
   at System.Net.Test.Common.Http3LoopbackStream.HandleRequestAsync(HttpStatusCode statusCode, IList`1 headers, String content) in /home/manicka/Repositories/runtime.2/src/libraries/Common/tests/System/Net/Http/Http3LoopbackStream.cs:line 48
   at System.Net.Http.Functional.Tests.HttpClientHandlerTest_Http3.<>c__DisplayClass5_1.<<ReservedFrameType_Throws>b__2>d.MoveNext() in /home/manicka/Repositories/runtime.2/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http3.cs:line 136
--- End of stack trace from previous location ---
----- Inner Stack Trace -----
   at System.Net.Test.Common.Http3LoopbackStream.ReadRequestDataAsync(Boolean readBody) in /home/manicka/Repositories/runtime.2/src/libraries/Common/tests/System/Net/Http/Http3LoopbackStream.cs:line 179
   at System.Net.Test.Common.Http3LoopbackStream.HandleRequestAsync(HttpStatusCode statusCode, IList`1 headers, String content) in /home/manicka/Repositories/runtime.2/src/libraries/Common/tests/System/Net/Http/Http3LoopbackStream.cs:line 48
   at System.Net.Http.Functional.Tests.HttpClientHandlerTest_Http3.<>c__DisplayClass5_1.<<ReservedFrameType_Throws>b__2>d.MoveNext() in /home/manicka/Repositories/runtime.2/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http3.cs:line 136
--- End of stack trace from previous location ---
Author: ManickaP
Assignees: -
Labels:

area-System.Net.Http, disabled-test

Milestone: 6.0.0

@CarnaViire
Copy link
Member

The error is now different (either OperationCanceledException or INVALID_STATE) -- described in #58078
Need to revisit after #58078 is fixed.

@CarnaViire CarnaViire removed their assignment Aug 27, 2021
@wfurt
Copy link
Member

wfurt commented Aug 27, 2021

BTW I was under impression that the INVALID_STATE should not happen any more with current msquic and we should get different error code when the connection is closed remotely. Do you still see it?

@CarnaViire
Copy link
Member

I've seen that on 8/24 on that day's runtime main. I will re-check later

     System.Net.Http.Functional.Tests.SocketsHttpHandlerTest_Http3_MsQuic.ReservedFrameType_Throws [FAIL]
      Assert.Throws() Failure
      Expected: typeof(System.Net.Quic.QuicConnectionAbortedException)
      Actual:   typeof(System.Net.Quic.QuicException): Could not send data to peer. Error Code: INVALID_STATE
      ---- System.Net.Quic.QuicException : Could not send data to peer. Error Code: INVALID_STATE
      Stack Trace:
        C:\Users\knatalia\dev\git\runtime\src\libraries\System.Net.Quic\src\System\Net\Quic\Implementations\MsQuic\Internal\QuicExceptionHelpers.cs(14,0): at System.Net.Quic.Implementations.MsQuic.Internal.QuicExceptionHelpers.ThrowIfFailed(UInt32 status, String message, Exception innerException)
        C:\Users\knatalia\dev\git\runtime\src\libraries\System.Net.Quic\src\System\Net\Quic\Implementations\MsQuic\MsQuicStream.cs(1209,0): at System.Net.Quic.Implementations.MsQuic.MsQuicStream.SendReadOnlyMemoryAsync(ReadOnlyMemory`1 buffer, QUIC_SEND_FLAGS flags)
        C:\Users\knatalia\dev\git\runtime\src\libraries\System.Net.Quic\src\System\Net\Quic\Implementations\MsQuic\MsQuicStream.cs(296,0): at System.Net.Quic.Implementations.MsQuic.MsQuicStream.WriteAsync(ReadOnlyMemory`1 buffer, Boolean endStream, CancellationToken cancellationToken)
        C:\Users\knatalia\dev\git\runtime\src\libraries\Common\tests\System\Net\Http\Http3LoopbackStream.cs(162,0): at System.Net.Test.Common.Http3LoopbackStream.SendFrameHeaderAsync(Int64 frameType, Int32 payloadLength)
        C:\Users\knatalia\dev\git\runtime\src\libraries\Common\tests\System\Net\Http\Http3LoopbackStream.cs(167,0): at System.Net.Test.Common.Http3LoopbackStream.SendFrameAsync(Int64 frameType, ReadOnlyMemory`1 framePayload)
        C:\Users\knatalia\dev\git\runtime\src\libraries\Common\tests\System\Net\Http\Http3LoopbackStream.cs(123,0): at System.Net.Test.Common.Http3LoopbackStream.SendHeadersFrameAsync(HttpStatusCode statusCode, IEnumerable`1 headers, Boolean qpackEncodeStatus)
        C:\Users\knatalia\dev\git\runtime\src\libraries\Common\tests\System\Net\Http\Http3LoopbackStream.cs(264,0): at System.Net.Test.Common.Http3LoopbackStream.SendResponseHeadersAsync(HttpStatusCode statusCode, IEnumerable`1 headers)
        C:\Users\knatalia\dev\git\runtime\src\libraries\Common\tests\System\Net\Http\Http3LoopbackStream.cs(247,0): at System.Net.Test.Common.Http3LoopbackStream.SendResponseAsync(HttpStatusCode statusCode, IList`1 headers, String content, Boolean isFinal)
        C:\Users\knatalia\dev\git\runtime\src\libraries\Common\tests\System\Net\Http\Http3LoopbackStream.cs(53,0): at System.Net.Test.Common.Http3LoopbackStream.HandleRequestAsync(HttpStatusCode statusCode, IList`1 headers, String content)
        C:\Users\knatalia\dev\git\runtime\src\libraries\System.Net.Http\tests\FunctionalTests\HttpClientHandlerTest.Http3.cs(299,0): at System.Net.Http.Functional.Tests.HttpClientHandlerTest_Http3.<>c__DisplayClass7_1.<<ReservedFrameType_Throws>b__2>d.MoveNext()
        --- End of stack trace from previous location ---
        ----- Inner Stack Trace -----
        C:\Users\knatalia\dev\git\runtime\src\libraries\System.Net.Quic\src\System\Net\Quic\Implementations\MsQuic\Internal\QuicExceptionHelpers.cs(14,0): at System.Net.Quic.Implementations.MsQuic.Internal.QuicExceptionHelpers.ThrowIfFailed(UInt32 status, String message, Exception innerException)
        C:\Users\knatalia\dev\git\runtime\src\libraries\System.Net.Quic\src\System\Net\Quic\Implementations\MsQuic\MsQuicStream.cs(1209,0): at System.Net.Quic.Implementations.MsQuic.MsQuicStream.SendReadOnlyMemoryAsync(ReadOnlyMemory`1 buffer, QUIC_SEND_FLAGS flags)
        C:\Users\knatalia\dev\git\runtime\src\libraries\System.Net.Quic\src\System\Net\Quic\Implementations\MsQuic\MsQuicStream.cs(296,0): at System.Net.Quic.Implementations.MsQuic.MsQuicStream.WriteAsync(ReadOnlyMemory`1 buffer, Boolean endStream, CancellationToken cancellationToken)
        C:\Users\knatalia\dev\git\runtime\src\libraries\Common\tests\System\Net\Http\Http3LoopbackStream.cs(162,0): at System.Net.Test.Common.Http3LoopbackStream.SendFrameHeaderAsync(Int64 frameType, Int32 payloadLength)
        C:\Users\knatalia\dev\git\runtime\src\libraries\Common\tests\System\Net\Http\Http3LoopbackStream.cs(167,0): at System.Net.Test.Common.Http3LoopbackStream.SendFrameAsync(Int64 frameType, ReadOnlyMemory`1 framePayload)
        C:\Users\knatalia\dev\git\runtime\src\libraries\Common\tests\System\Net\Http\Http3LoopbackStream.cs(123,0): at System.Net.Test.Common.Http3LoopbackStream.SendHeadersFrameAsync(HttpStatusCode statusCode, IEnumerable`1 headers, Boolean qpackEncodeStatus)
        C:\Users\knatalia\dev\git\runtime\src\libraries\Common\tests\System\Net\Http\Http3LoopbackStream.cs(264,0): at System.Net.Test.Common.Http3LoopbackStream.SendResponseHeadersAsync(HttpStatusCode statusCode, IEnumerable`1 headers)
        C:\Users\knatalia\dev\git\runtime\src\libraries\Common\tests\System\Net\Http\Http3LoopbackStream.cs(247,0): at System.Net.Test.Common.Http3LoopbackStream.SendResponseAsync(HttpStatusCode statusCode, IList`1 headers, String content, Boolean isFinal)
        C:\Users\knatalia\dev\git\runtime\src\libraries\Common\tests\System\Net\Http\Http3LoopbackStream.cs(53,0): at System.Net.Test.Common.Http3LoopbackStream.HandleRequestAsync(HttpStatusCode statusCode, IList`1 headers, String content)
        C:\Users\knatalia\dev\git\runtime\src\libraries\System.Net.Http\tests\FunctionalTests\HttpClientHandlerTest.Http3.cs(299,0): at System.Net.Http.Functional.Tests.HttpClientHandlerTest_Http3.<>c__DisplayClass7_1.<<ReservedFrameType_Throws>b__2>d.MoveNext()
        --- End of stack trace from previous location ---

@wfurt
Copy link
Member

wfurt commented Aug 30, 2021

OK. I was expecting we would no longer see this after #57541 as that should bring microsoft/msquic#1875
cc: @ThadHouse @nibanks

@ThadHouse
Copy link
Contributor

You can still get invalid state if you have closed the connection locally. Are you sure that hasn't happened?

@wfurt
Copy link
Member

wfurt commented Aug 30, 2021

I would expect not but @CarnaViire to confirm. WriteAsync should not try to call msquic if locally closed.

@CarnaViire
Copy link
Member

I was looking at microsoft/msquic#1875 and it seems to fix that only for StreamStart and StreamOpen, but not for StreamSend. Is it possible INVALID_STATE is still used for StreamSend? @nibanks @thhous-msft

@CarnaViire
Copy link
Member

I confirm that I can still get INVALID_STATE because of the race between calling StreamSend and receiving SHUTDOWN_INITIATED_BY_PEER.

The test is to call StreamSend indefinitely while peer is calling ConnectionClose after a couple of reads.

  1. Most of the time I would get SEND_COMPLETE with Canceled=true. We have another issue to correctly handle it QUIC: Peer closing connection can result in OperationCanceledException on pending write #58078
  2. Sometimes I would luck out and receive stream's SHUTDOWN_COMPLETE with ConnectionShutdown=true.
  3. And sometimes StreamSend itself will fail with INVALID_STATE.
    Extract from msquic logs below (I have full logs if needed)
115964 [1]42FC.3134::2022/02/16-17:34:34.879798900 [Microsoft-Quic][ api] Enter 14(CONNECTION_SHUTDOWN) (0x23C2D306D20).
115966 [1]42FC.3134::2022/02/16-17:34:34.879800500 [Microsoft-Quic][ api] Exit
116007 [7]42FC.2B50::2022/02/16-17:34:34.879836900 [Microsoft-Quic][conn][0x23C2D306D20] App Shutdown: 123 (Remote=0)

116010 [7]42FC.2B50::2022/02/16-17:34:34.879839400 [Microsoft-Quic][strm][0x23C2CF088F0] Indicating QUIC_STREAM_EVENT_SEND_SHUTDOWN_COMPLETE
116014 [7]42FC.2B50::2022/02/16-17:34:34.879844400 [Microsoft-Quic][strm][0x23C2CF088F0] Send State: 3(RESET_ACKED)
116015 [7]42FC.2B50::2022/02/16-17:34:34.879845500 [Microsoft-Quic][strm][0x23C2CF088F0] Recv State: 3(STOPPED)
116018 [7]42FC.2B50::2022/02/16-17:34:34.879848000 [Microsoft-Quic][strm][0x23C2CF088F0] Indicating QUIC_STREAM_EVENT_SHUTDOWN_COMPLETE [ConnectionShutdown=1]

116021 [1]42FC.3134::2022/02/16-17:34:34.879850800 [Microsoft-Quic][ api] Enter 22(STREAM_SEND) (0x23C2CF07AE0).
116023 [1]42FC.3134::2022/02/16-17:34:34.879852300 [Microsoft-Quic][ api] Exit 459749
116084 [4]42FC.C1C0::2022/02/16-17:34:34.879964300 [Microsoft-Quic][strm][0x23C2CF07AE0] Indicating QUIC_STREAM_EVENT_SEND_COMPLETE [0000023C2B64C330]

116237 [4]42FC.3134::2022/02/16-17:34:34.880321100 [Microsoft-Quic][ api] Enter 22(STREAM_SEND) (0x23C2CF07AE0).
116239 [4]42FC.3134::2022/02/16-17:34:34.880325800 [Microsoft-Quic][ api] Exit 459749

116251 [7]42FC.2B50::2022/02/16-17:34:34.880385600 [Microsoft-Quic][strm][0x23C2CF088F0] Ignoring recv flush (recv disabled)

116256 [1]42FC.C1C0::2022/02/16-17:34:34.880389400 [Microsoft-Quic][strm][0x23C2CF07AE0] Indicating QUIC_STREAM_EVENT_SEND_COMPLETE [0000023C2B64C8D0]

116310 [4]42FC.25A8::2022/02/16-17:34:34.880446300 [Microsoft-Quic][ api] Enter 22(STREAM_SEND) (0x23C2CF07AE0).
116314 [4]42FC.25A8::2022/02/16-17:34:34.880451300 [Microsoft-Quic][ api] Exit 459749
116335 [1]42FC.C1C0::2022/02/16-17:34:34.880479400 [Microsoft-Quic][strm][0x23C2CF07AE0] Indicating QUIC_STREAM_EVENT_SEND_COMPLETE [0000023C2B64C510]

116340 [4]42FC.25A8::2022/02/16-17:34:34.880484600 [Microsoft-Quic][ api] Enter 22(STREAM_SEND) (0x23C2CF07AE0).
116342 [4]42FC.25A8::2022/02/16-17:34:34.880485800 [Microsoft-Quic][ api] Exit 459749
116392 [1]42FC.C1C0::2022/02/16-17:34:34.880541200 [Microsoft-Quic][strm][0x23C2CF07AE0] Indicating QUIC_STREAM_EVENT_SEND_COMPLETE [0000023C2B64CC90]

116430 [1]42FC.C1C0::2022/02/16-17:34:34.880588500 [Microsoft-Quic][conn][0x23C2B64CEE0] Indicating QUIC_CONNECTION_EVENT_SHUTDOWN_INITIATED_BY_PEER [0x7b]

116434 [4]42FC.3134::2022/02/16-17:34:34.880596700 [Microsoft-Quic][ api] Enter 22(STREAM_SEND) (0x23C2CF07AE0).
116435 [4]42FC.3134::2022/02/16-17:34:34.880598300 [Microsoft-Quic][ api] Exit 2147947423        <--- INVALID_STATE

116436 [1]42FC.C1C0::2022/02/16-17:34:34.880598900 [Microsoft-Quic][strm][0x23C2CF07AE0] Indicating QUIC_STREAM_EVENT_SEND_SHUTDOWN_COMPLETE
116441 [1]42FC.C1C0::2022/02/16-17:34:34.880606400 [Microsoft-Quic][strm][0x23C2CF07AE0] Indicating QUIC_STREAM_EVENT_SHUTDOWN_COMPLETE [ConnectionShutdown=1]

cc @nibanks @thhous-msft

It would be great if we could distinguish INVALID_STATE from connection shutdown vs all other possible invalid states.

Same with Canceled. If the send is canceled because of connection shutdown, we can ignore the event and wait for SHUTDOWN_COMPLETE, but if it's a user cancelation of Send operation, we should throw OperationCanceledException.

@thhous-msft
Copy link

StreamSend wasn't part of the changes done before, as the bug we got seemed to only apply to Start. I'll look into StreamSend.

@nibanks
Copy link

nibanks commented Feb 16, 2022

@CarnaViire would you mind opening a new MsQuic Issue to track StreamSend and we'll prioritize accordingly. We've still got a number of v2 things that need to get fixed ASAP. This can be fixed in a new minor version later.

@CarnaViire
Copy link
Member

opening a new MsQuic Issue

Sure! We are currently in the middle of collecting all the potential problems and asks to discuss with you and then convert some or all of them into issues.

@CarnaViire
Copy link
Member

Judging from the discussion here #55485 (comment) is it correct that if SEND_COMPLETE contains Canceled=true, that could only mean that the stream is being closed (because of connection shutdown or other reasons)? Meaning, can we skip that event and wait for the following STREAM_SHUTDOWN* event? @nibanks @thhous-msft

@nibanks
Copy link

nibanks commented Feb 16, 2022

A canceled send can happen for one of the following reasons:

  1. The peer aborted their receive.
  2. The local app aborted their send.
  3. The connection was shutdown (by either side).

@rzikm rzikm self-assigned this Apr 6, 2022
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Apr 6, 2022
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Apr 8, 2022
@ghost ghost locked as resolved and limited conversation to collaborators May 8, 2022
@karelz karelz modified the milestones: Future, 7.0.0 Jul 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Net.Http disabled-test The test is disabled in source code against the issue
Projects
None yet
8 participants