Skip to content

Commit 2ea2729

Browse files
committed
Fix potential crash
1 parent 1eac511 commit 2ea2729

File tree

1 file changed

+15
-11
lines changed

1 file changed

+15
-11
lines changed

src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs

+15-11
Original file line numberDiff line numberDiff line change
@@ -541,12 +541,12 @@ private unsafe int HandleEventShutdownInitiatedByTransport(ref SHUTDOWN_INITIATE
541541
{
542542
Exception exception = ExceptionDispatchInfo.SetCurrentStackTrace(ThrowHelper.GetExceptionForMsQuicStatus(data.Status, (long)data.ErrorCode));
543543
_connectedTcs.TrySetException(exception);
544-
CompleteAndDrainAcceptQueue(exception);
544+
CompleteAcceptQueue(exception, false);
545545
return QUIC_STATUS_SUCCESS;
546546
}
547547
private unsafe int HandleEventShutdownInitiatedByPeer(ref SHUTDOWN_INITIATED_BY_PEER_DATA data)
548548
{
549-
CompleteAndDrainAcceptQueue(ExceptionDispatchInfo.SetCurrentStackTrace(ThrowHelper.GetConnectionAbortedException((long)data.ErrorCode)));
549+
CompleteAcceptQueue(ExceptionDispatchInfo.SetCurrentStackTrace(ThrowHelper.GetConnectionAbortedException((long)data.ErrorCode)), false);
550550
return QUIC_STATUS_SUCCESS;
551551
}
552552
private unsafe int HandleEventShutdownComplete()
@@ -555,7 +555,7 @@ private unsafe int HandleEventShutdownComplete()
555555
_tlsSecret?.WriteSecret();
556556

557557
Exception exception = ExceptionDispatchInfo.SetCurrentStackTrace(_disposed == 1 ? new ObjectDisposedException(GetType().FullName) : ThrowHelper.GetOperationAbortedException());
558-
CompleteAndDrainAcceptQueue(exception);
558+
CompleteAcceptQueue(exception, true);
559559
_connectedTcs.TrySetException(exception);
560560
_shutdownTokenSource.Cancel();
561561
_shutdownTcs.TrySetResult(final: true);
@@ -666,22 +666,26 @@ private static unsafe int NativeCallback(QUIC_HANDLE* connection, void* context,
666666
}
667667
}
668668

669-
private void CompleteAndDrainAcceptQueue(Exception? ex)
669+
private void CompleteAcceptQueue(Exception? ex, bool drain)
670670
{
671-
if (_acceptQueue.Writer.TryComplete(ex))
671+
_acceptQueue.Writer.TryComplete(ex);
672+
673+
if (drain)
672674
{
673-
// also drain the queue. Because stream shutdown events are indicated before connection shutdown,
674-
// the QuicStream instances have already been signaled and closed internally. We only need to dispose them,
675-
// which in this situation should complete synchronously.
675+
// This should be only called after connection SHUTDOWN_COMPLETE has been indicated.
676+
// At that point, all streams have been already shut down internally and we need
677+
// only to close the handle via dispose, so DisposeAsync below should complete
678+
// synchronously (which is necessary for this method to be callable from MsQuic
679+
// event callback).
676680
while (_acceptQueue.Reader.TryRead(out QuicStream? stream))
677681
{
678682
ValueTask task = stream.DisposeAsync();
679683
Debug.Assert(task.IsCompletedSuccessfully);
680684
task.GetAwaiter().GetResult();
681685
}
682-
}
683686

684-
Debug.Assert(_acceptQueue.Reader.Completion.IsCompleted);
687+
Debug.Assert(_acceptQueue.Reader.Completion.IsCompleted);
688+
}
685689
}
686690

687691
/// <summary>
@@ -737,6 +741,6 @@ public async ValueTask DisposeAsync()
737741
}
738742

739743
// Flush the queue and dispose all remaining streams.
740-
CompleteAndDrainAcceptQueue(ExceptionDispatchInfo.SetCurrentStackTrace(new ObjectDisposedException(GetType().FullName)));
744+
CompleteAcceptQueue(ExceptionDispatchInfo.SetCurrentStackTrace(new ObjectDisposedException(GetType().FullName)), true);
741745
}
742746
}

0 commit comments

Comments
 (0)