Fix data race leading to a deadlock when opening QuicStream#101250
Fix data race leading to a deadlock when opening QuicStream#101250rzikm merged 7 commits intodotnet:mainfrom
Conversation
|
Tagging subscribers to this area: @dotnet/ncl |
src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http3.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.Quic/src/System/Net/Quic/QuicStream.cs
Outdated
Show resolved
Hide resolved
ManickaP
left a comment
There was a problem hiding this comment.
Looks good, thanks for finding and fixing this!
src/libraries/System.Net.Quic/src/System/Net/Quic/QuicStream.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.Quic/src/System/Net/Quic/QuicStream.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs
Outdated
Show resolved
Hide resolved
|
The original version was not a complete fix, it turns out that calling runtime/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs Lines 433 to 436 in 79a7156 So I added logic which will drain the channel once we are sure the streams inside have been shutdown. Please take another look. |
src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs
Outdated
Show resolved
Hide resolved
|
/azp run runtime-libraries-coreclr outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs
Outdated
Show resolved
Hide resolved
69d4dc1 to
5586d18
Compare
|
no QUIC or HTTP/3 failures in CI. |
…01250) * Fix data race leading to a deadlock. * Remove unwanted change * Code review feedback * Fix hang * Add assert * Fix potential crash * Code review feedback
…01250) * Fix data race leading to a deadlock. * Remove unwanted change * Code review feedback * Fix hang * Add assert * Fix potential crash * Code review feedback
|
/backport to release/8.0-staging |
|
Started backporting to release/8.0-staging: https://github.com/dotnet/runtime/actions/runs/9059881753 |
|
@rzikm backporting to release/8.0-staging failed, the patch most likely resulted in conflicts: $ git am --3way --ignore-whitespace --keep-non-patch changes.patch
Applying: Fix data race leading to a deadlock.
Using index info to reconstruct a base tree...
M src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http3.cs
M src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs
M src/libraries/System.Net.Quic/src/System/Net/Quic/QuicStream.cs
Falling back to patching base and 3-way merge...
Auto-merging src/libraries/System.Net.Quic/src/System/Net/Quic/QuicStream.cs
Auto-merging src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs
CONFLICT (content): Merge conflict in src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs
Auto-merging src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http3.cs
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Fix data race leading to a deadlock.
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Error: The process '/usr/bin/git' failed with exit code 128Please backport manually! |
|
@rzikm an error occurred while backporting to release/8.0-staging, please check the run log for details! Error: git am failed, most likely due to a merge conflict. |
…01250) * Fix data race leading to a deadlock. * Remove unwanted change * Code review feedback * Fix hang * Add assert * Fix potential crash * Code review feedback
…01250) * Fix data race leading to a deadlock. * Remove unwanted change * Code review feedback * Fix hang * Add assert * Fix potential crash * Code review feedback
Fixes #101233.
Fixes #100971.
Contributes to #101463
See original issue for the description of the deadlock scenario.