From 740581cce911de2754375863188a3c3b2c5c9130 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Mon, 23 Aug 2021 09:09:17 -0600 Subject: [PATCH] [release/6.0] Fix QUIC ConnectionState NRE in HandleEventConnectionClose (#57742) * Fix QUIC ConnectionState NRE in HandleEventConnectionClose * Remove unnecessary Dispose Co-authored-by: Natalia Kondratyeva --- .../Quic/Implementations/MsQuic/MsQuicStream.cs | 33 +++++++++++----------- 1 file changed, 16 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 bbaf9c4..2624b1ce 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 @@ namespace System.Net.Quic.Implementations.MsQuic // 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 @@ namespace System.Net.Quic.Implementations.MsQuic 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,14 @@ namespace System.Net.Quic.Implementations.MsQuic { Debug.Assert(connectionState.Handle != null); + if (!connectionState.TryAddStream(this)) + { + 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 +178,6 @@ namespace System.Net.Quic.Implementations.MsQuic 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()) { -- 2.7.4