From ab1dae211090cb86d68de1e6f1d606787d50ccba Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Wed, 6 Apr 2022 17:41:15 +0200 Subject: [PATCH 1/2] Fix exception thrown from MsQuicStream writes when connection is aborted by peer. --- .../Quic/Implementations/MsQuic/MsQuicStream.cs | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Net.Quic/src/System/Net/Quic/Implementations/MsQuic/MsQuicStream.cs b/src/libraries/System.Net.Quic/src/System/Net/Quic/Implementations/MsQuic/MsQuicStream.cs index 73ea871d3e160..729972edc2bea 100644 --- a/src/libraries/System.Net.Quic/src/System/Net/Quic/Implementations/MsQuic/MsQuicStream.cs +++ b/src/libraries/System.Net.Quic/src/System/Net/Quic/Implementations/MsQuic/MsQuicStream.cs @@ -1264,9 +1264,9 @@ private static uint HandleEventPeerSendShutdown(State state) private static uint HandleEventSendComplete(State state, ref StreamEvent evt) { StreamEventDataSendComplete sendCompleteEvent = evt.Data.SendComplete; - bool canceled = sendCompleteEvent.Canceled != 0; bool complete = false; + Exception? abortException = null; lock (state) { @@ -1276,9 +1276,18 @@ private static uint HandleEventSendComplete(State state, ref StreamEvent evt) complete = true; } - if (canceled) + if (sendCompleteEvent.Canceled != 0) { state.SendState = SendState.Aborted; + + // + // There are multiple reasons the send could have been cancelled: + // - Connection was aborted (either by transport or peer) => error-code already provided on the connection-level event + // - Stream's receive side was aborted by peer => already handled by HandleEventPeerRecvAborted + // and we will not set the exception due to complete == false + // - Stream's send side was aborted locally => no connection-level abort code and we return QuicOperationAbortException + // + abortException = ThrowHelper.GetConnectionAbortedException(state.ConnectionState.AbortErrorCode); } } @@ -1286,14 +1295,14 @@ private static uint HandleEventSendComplete(State state, ref StreamEvent evt) { CleanupSendState(state); - if (!canceled) + if (abortException == null) { state.SendResettableCompletionSource.Complete(MsQuicStatusCodes.Success); } else { state.SendResettableCompletionSource.CompleteException( - ExceptionDispatchInfo.SetCurrentStackTrace(new OperationCanceledException("Write was canceled"))); + ExceptionDispatchInfo.SetCurrentStackTrace(abortException)); } } From 8e868c1b8e23308d0995787bc48de96023600e83 Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Thu, 7 Apr 2022 16:13:21 +0200 Subject: [PATCH 2/2] Code review feedback --- .../Implementations/MsQuic/MsQuicStream.cs | 25 +++++++++---------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/src/libraries/System.Net.Quic/src/System/Net/Quic/Implementations/MsQuic/MsQuicStream.cs b/src/libraries/System.Net.Quic/src/System/Net/Quic/Implementations/MsQuic/MsQuicStream.cs index 729972edc2bea..baf9b70bf8452 100644 --- a/src/libraries/System.Net.Quic/src/System/Net/Quic/Implementations/MsQuic/MsQuicStream.cs +++ b/src/libraries/System.Net.Quic/src/System/Net/Quic/Implementations/MsQuic/MsQuicStream.cs @@ -1264,9 +1264,9 @@ private static uint HandleEventPeerSendShutdown(State state) private static uint HandleEventSendComplete(State state, ref StreamEvent evt) { StreamEventDataSendComplete sendCompleteEvent = evt.Data.SendComplete; + bool canceled = sendCompleteEvent.Canceled != 0; bool complete = false; - Exception? abortException = null; lock (state) { @@ -1276,18 +1276,9 @@ private static uint HandleEventSendComplete(State state, ref StreamEvent evt) complete = true; } - if (sendCompleteEvent.Canceled != 0) + if (canceled) { state.SendState = SendState.Aborted; - - // - // There are multiple reasons the send could have been cancelled: - // - Connection was aborted (either by transport or peer) => error-code already provided on the connection-level event - // - Stream's receive side was aborted by peer => already handled by HandleEventPeerRecvAborted - // and we will not set the exception due to complete == false - // - Stream's send side was aborted locally => no connection-level abort code and we return QuicOperationAbortException - // - abortException = ThrowHelper.GetConnectionAbortedException(state.ConnectionState.AbortErrorCode); } } @@ -1295,14 +1286,22 @@ private static uint HandleEventSendComplete(State state, ref StreamEvent evt) { CleanupSendState(state); - if (abortException == null) + if (!canceled) { state.SendResettableCompletionSource.Complete(MsQuicStatusCodes.Success); } else { + // + // There are multiple reasons the send could have been cancelled: + // - Connection was aborted (either by transport or peer) => error-code already provided on the connection-level event + // - Stream's receive side was aborted by peer => already handled by HandleEventPeerRecvAborted + // and we will not set the exception due to complete == false + // - Stream's send side was aborted locally => no connection-level abort code and we return QuicOperationAbortException + // state.SendResettableCompletionSource.CompleteException( - ExceptionDispatchInfo.SetCurrentStackTrace(abortException)); + ExceptionDispatchInfo.SetCurrentStackTrace( + ThrowHelper.GetConnectionAbortedException(state.ConnectionState.AbortErrorCode))); } }