[H/3] Remove waiting on tasks from H3 request stream dispose #92410
Conversation
|
Tagging subscribers to this area: @dotnet/ncl Issue DetailsThe waiting on tasks was added due to HTTP stress crashing on runtime/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicStream.cs Lines 716 to 717 in 9c8ff4c H3 stream is issuing reads and writes on QuicStream in parallel. So if one ends on cancellation and decides to dispose the stream, the DisposeAsync might run in parallel with pending Write/Read. Basically, this change reverts these to commits: d6f5061, f6261e9 from #90253. Fixes #92390 The asserts still need to addressed, but we don't have a running stress atm.
|
|
I've run locally all the tests from dotnet/aspnetcore#50833 with the fix and they pass promptly, no timeout, no hangs. |
|
/azp run runtime-libraries-coreclr outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run runtime-libraries-coreclr outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| Debug.Assert(_receiveTcs.IsCompleted); | ||
| Debug.Assert(_sendTcs.IsCompleted); | ||
| // TODO: If there was and ongoing read/write when dispose was called, MsQuic events might be processed, but the continuations may not thus the state can still be in Ready. | ||
| //Debug.Assert(_receiveTcs.IsCompleted); |
There was a problem hiding this comment.
I'm not bug fan of submitting comments like this. same bellow.
ghost
locked as resolved and limited conversation to collaborators
The waiting on tasks was added due to HTTP stress crashing on
QuicStreamasserts:runtime/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicStream.cs
Lines 716 to 717 in 9c8ff4c
H3 stream is issuing reads and writes on QuicStream in parallel. So if one ends on cancellation and decides to dispose the stream, the DisposeAsync might run in parallel with pending Write/Read.
Basically, this change reverts these to commits: d6f5061, f6261e9 from #90253.
Fixes #92390
The asserts still need to addressed, but we don't have a running stress atm.
cc @JamesNK