From 1d4f8f4115c5901c49d3f3cb46c708c1afc1463e Mon Sep 17 00:00:00 2001 From: Natalia Kondratyeva Date: Wed, 18 Aug 2021 17:54:29 +0200 Subject: [PATCH 1/2] Fix QUIC ConnectionState NRE in HandleEventConnectionClose --- .../Implementations/MsQuic/MsQuicStream.cs | 34 +++++++++---------- 1 file changed, 17 insertions(+), 17 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 bbaf9c4ed41bb7..c27815a96b084b 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 @@ -95,6 +95,14 @@ public void Cleanup() // inbound. internal MsQuicStream(MsQuicConnection.State connectionState, SafeMsQuicStreamHandle streamHandle, QUIC_STREAM_OPEN_FLAGS flags) { + if (!connectionState.TryAddStream(this)) + { + throw new ObjectDisposedException(nameof(QuicConnection)); + } + // this assignment should be done before SetCallbackHandlerDelegate to prevent NRE in HandleEventConnectionClose + // but after TryAddStream to prevent unnecessary RemoveStream in finalizer + _state.ConnectionState = connectionState; + _state.Handle = streamHandle; _canRead = true; _canWrite = !flags.HasFlag(QUIC_STREAM_OPEN_FLAGS.UNIDIRECTIONAL); @@ -117,14 +125,6 @@ internal MsQuicStream(MsQuicConnection.State connectionState, SafeMsQuicStreamHa throw; } - if (!connectionState.TryAddStream(this)) - { - _state.StateGCHandle.Free(); - throw new ObjectDisposedException(nameof(QuicConnection)); - } - - _state.ConnectionState = connectionState; - _state.TraceId = MsQuicTraceHelper.GetTraceId(_state.Handle); if (NetEventSource.Log.IsEnabled()) { @@ -140,6 +140,15 @@ internal MsQuicStream(MsQuicConnection.State connectionState, QUIC_STREAM_OPEN_F { Debug.Assert(connectionState.Handle != null); + if (!connectionState.TryAddStream(this)) + { + _state.Handle?.Dispose(); + throw new ObjectDisposedException(nameof(QuicConnection)); + } + // this assignment should be done before StreamOpenDelegate to prevent NRE in HandleEventConnectionClose + // but after TryAddStream to prevent unnecessary RemoveStream in finalizer + _state.ConnectionState = connectionState; + _canRead = !flags.HasFlag(QUIC_STREAM_OPEN_FLAGS.UNIDIRECTIONAL); _canWrite = true; @@ -170,15 +179,6 @@ internal MsQuicStream(MsQuicConnection.State connectionState, QUIC_STREAM_OPEN_F throw; } - if (!connectionState.TryAddStream(this)) - { - _state.Handle?.Dispose(); - _state.StateGCHandle.Free(); - throw new ObjectDisposedException(nameof(QuicConnection)); - } - - _state.ConnectionState = connectionState; - _state.TraceId = MsQuicTraceHelper.GetTraceId(_state.Handle); if (NetEventSource.Log.IsEnabled()) { From 450462d0f9e0c155387851c3588e028ccacf4f7a Mon Sep 17 00:00:00 2001 From: Natalia Kondratyeva Date: Wed, 18 Aug 2021 18:22:18 +0200 Subject: [PATCH 2/2] Remove unnecessary Dispose --- .../src/System/Net/Quic/Implementations/MsQuic/MsQuicStream.cs | 1 - 1 file changed, 1 deletion(-) 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 c27815a96b084b..2624b1ce2ea94c 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 @@ -142,7 +142,6 @@ internal MsQuicStream(MsQuicConnection.State connectionState, QUIC_STREAM_OPEN_F if (!connectionState.TryAddStream(this)) { - _state.Handle?.Dispose(); throw new ObjectDisposedException(nameof(QuicConnection)); } // this assignment should be done before StreamOpenDelegate to prevent NRE in HandleEventConnectionClose