HTTP/3: Support QUIC idle timeout more cleanly

src/Servers/Kestrel/Core/src/CoreStrings.resx

@@ -713,4 +713,7 @@ For more information on configuring HTTPS see https://go.microsoft.com/fwlink/?l
   <data name="NonzeroContentLengthNotAllowedOn205" xml:space="preserve">
     <value>Responses with status code 205 cannot have a non-zero Content-Length value.</value>
   </data>
+  <data name="Http3ErrorControlStreamClosed" xml:space="preserve">
+    <value>A control stream used by the connection was closed or reset.</value>
+  </data>
 </root>

src/Servers/Kestrel/Core/src/Internal/Http3/Http3Connection.cs

@@ -198,9 +198,49 @@ public void Tick(DateTimeOffset now)
             return;
         }
 
+        ValidateOpenControlStreams(now);
         UpdateStreamTimeouts(now);
     }
 
+    private void ValidateOpenControlStreams(DateTimeOffset now)
+    {
+        var ticks = now.Ticks;
+
+        // This method validates that a connnection's control streams are open.
+        //
+        // They're checked on a delayed timer because when a connection is aborted or timed out, notifications are sent to open streams
+        // and the connection simultaneously. This is a problem because when a control stream is closed the connection should be aborted
+        // with the H3_CLOSED_CRITICAL_STREAM status. There is a race between the connection closing for the real reason, and control
+        // streams closing the connection with H3_CLOSED_CRITICAL_STREAM.
+        //
+        // Realistically, control streams are never closed except when the connection is. A small delay in aborting the connection in the
+        // unlikely situation where a control stream is incorrectly closed should be fine.
+        ValidateOpenControlStream(OutboundControlStream, this, ticks);
+        ValidateOpenControlStream(ControlStream, this, ticks);
+        ValidateOpenControlStream(EncoderStream, this, ticks);
+        ValidateOpenControlStream(DecoderStream, this, ticks);
+
+        static void ValidateOpenControlStream(Http3ControlStream? stream, Http3Connection connection, long ticks)
+        {
+            if (stream != null)
+            {
+                if (stream.IsCompleted || stream.IsAborted || stream.EndStreamReceived)
+                {
+                    // If a control stream is no longer active then set a timeout so that the connection is aborted next tick.
+                    if (stream.StreamTimeoutTicks == default)
+                    {
+                        stream.StreamTimeoutTicks = ticks;
+                    }
+
+                    if (stream.StreamTimeoutTicks < ticks)
+                    {
+                        connection.OnStreamConnectionError(new Http3ConnectionErrorException("A control stream used by the connection was closed or reset.", Http3ErrorCode.ClosedCriticalStream));
+                    }
+                }
+            }
+        }
+    }
+
     private void UpdateStreamTimeouts(DateTimeOffset now)
     {
         // This method checks for timeouts:
@@ -271,7 +311,7 @@ private void UpdateStreamTimeouts(DateTimeOffset now)
                     {
                         // Cancel connection to be consistent with other data rate limits.
                         Log.ResponseMinimumDataRateNotSatisfied(_context.ConnectionId, stream.TraceIdentifier);
-                        Abort(new ConnectionAbortedException(CoreStrings.ConnectionTimedBecauseResponseMininumDataRateNotSatisfied), Http3ErrorCode.InternalError);
+                        OnStreamConnectionError(new Http3ConnectionErrorException(CoreStrings.ConnectionTimedBecauseResponseMininumDataRateNotSatisfied, Http3ErrorCode.InternalError));
                     }
                 }
             }
@@ -651,8 +691,7 @@ private async ValueTask ProcessOutboundControlStreamAsync(Http3ControlStream con
     {
         try
         {
-            await controlStream.SendStreamIdAsync(id: 0);
-            await controlStream.SendSettingsFrameAsync();
+            await controlStream.ProcessOutboundSendsAsync(id: 0);
         }
         catch (Exception ex)
         {
@@ -778,6 +817,11 @@ void IHttp3StreamLifetimeHandler.OnStreamCompleted(IHttp3Stream stream)
     }
 
     void IHttp3StreamLifetimeHandler.OnStreamConnectionError(Http3ConnectionErrorException ex)
+    {
+        OnStreamConnectionError(ex);
+    }
+
+    private void OnStreamConnectionError(Http3ConnectionErrorException ex)
     {
         Log.Http3ConnectionError(ConnectionId, ex);
         Abort(new ConnectionAbortedException(ex.Message, ex), ex.ErrorCode);

src/Servers/Kestrel/Core/src/Internal/Http3/Http3ControlStream.cs

@@ -29,9 +29,14 @@ internal abstract class Http3ControlStream : IHttp3Stream, IThreadPoolWorkItem
     private volatile int _isClosed;
     private long _headerType;
     private int _gracefulCloseInitiator;
-    private bool _connectionClosed;
+    private readonly object _completionLock = new();
 
     private bool _haveReceivedSettingsFrame;
+    private StreamCompletionFlags _completionState;
+
+    public bool EndStreamReceived => (_completionState & StreamCompletionFlags.EndStreamReceived) == StreamCompletionFlags.EndStreamReceived;
+    public bool IsAborted => (_completionState & StreamCompletionFlags.Aborted) == StreamCompletionFlags.Aborted;
+    public bool IsCompleted => (_completionState & StreamCompletionFlags.Completed) == StreamCompletionFlags.Completed;
 
     public long StreamId => _streamIdFeature.StreamId;
 
@@ -59,8 +64,7 @@ public Http3ControlStream(Http3StreamContext context, long? headerType)
 
     private void OnStreamClosed()
     {
-        Abort(new ConnectionAbortedException("HTTP_CLOSED_CRITICAL_STREAM"), Http3ErrorCode.InternalError);
-        _connectionClosed = true;
+        ApplyCompletionFlag(StreamCompletionFlags.Completed);
     }
 
     public PipeReader Input => _context.Transport.Input;
@@ -74,15 +78,27 @@ private void OnStreamClosed()
 
     public void Abort(ConnectionAbortedException abortReason, Http3ErrorCode errorCode)
     {
-        // TODO - Should there be a check here to track abort state to avoid
-        // running twice for a request?
+        lock (_completionLock)
+        {
+            if (IsCompleted || IsAborted)
+            {
+                return;
+            }
+
+            var (oldState, newState) = ApplyCompletionFlag(StreamCompletionFlags.Aborted);
+
+            if (oldState == newState)
+            {
+                return;
+            }
 
-        Log.Http3StreamAbort(_context.ConnectionId, errorCode, abortReason);
+            Log.Http3StreamAbort(_context.ConnectionId, errorCode, abortReason);
 
-        _errorCodeFeature.Error = (long)errorCode;
-        _frameWriter.Abort(abortReason);
+            _errorCodeFeature.Error = (long)errorCode;
+            _frameWriter.Abort(abortReason);
 
-        Input.Complete(abortReason);
+            Input.Complete(abortReason);
+        }
     }
 
     public void OnInputOrOutputCompleted()
@@ -101,9 +117,27 @@ private bool TryClose()
         return false;
     }
 
-    internal async ValueTask SendStreamIdAsync(long id)
+    private (StreamCompletionFlags OldState, StreamCompletionFlags NewState) ApplyCompletionFlag(StreamCompletionFlags completionState)
+    {
+        lock (_completionLock)
+        {
+            var oldCompletionState = _completionState;
+            _completionState |= completionState;
+
+            return (oldCompletionState, _completionState);
+        }
+    }
+
+    internal async ValueTask ProcessOutboundSendsAsync(long id)
     {
+        _streamClosedFeature.OnClosed(static state =>
+        {
+            var stream = (Http3ControlStream)state!;
+            stream.OnStreamClosed();
+        }, this);
+
         await _frameWriter.WriteStreamIdAsync(id);
+        await _frameWriter.WriteSettingsAsync(_serverPeerSettings.GetNonProtocolDefaults());
     }
 
     internal ValueTask<FlushResult> SendGoAway(long id)
@@ -112,11 +146,6 @@ internal ValueTask<FlushResult> SendGoAway(long id)
         return _frameWriter.WriteGoAway(id);
     }
 
-    internal async ValueTask SendSettingsFrameAsync()
-    {
-        await _frameWriter.WriteSettingsAsync(_serverPeerSettings.GetNonProtocolDefaults());
-    }
-
     private async ValueTask<long> TryReadStreamHeaderAsync()
     {
         // https://quicwg.org/base-drafts/draft-ietf-quic-http.html#section-6.2
@@ -212,6 +241,7 @@ public async Task ProcessRequestAsync<TContext>(IHttpApplication<TContext> appli
         }
         finally
         {
+            ApplyCompletionFlag(StreamCompletionFlags.Completed);
             _context.StreamLifetimeHandler.OnStreamCompleted(this);
         }
     }
@@ -241,12 +271,6 @@ private async Task HandleControlStream()
 
                 if (result.IsCompleted)
                 {
-                    if (!_connectionClosed)
-                    {
-                        // https://quicwg.org/base-drafts/draft-ietf-quic-http.html#section-6.2.1-2
-                        throw new Http3ConnectionErrorException(CoreStrings.Http3ErrorControlStreamClientClosedInbound, Http3ErrorCode.ClosedCriticalStream);
-                    }
-
                     return;
                 }
             }

src/Servers/Kestrel/Core/src/Internal/Http3/Http3Stream.cs

@@ -62,8 +62,8 @@ internal abstract partial class Http3Stream : HttpProtocol, IHttp3Stream, IHttpS
     protected readonly Http3RawFrame _incomingFrame = new();
 
     public bool EndStreamReceived => (_completionState & StreamCompletionFlags.EndStreamReceived) == StreamCompletionFlags.EndStreamReceived;
-    private bool IsAborted => (_completionState & StreamCompletionFlags.Aborted) == StreamCompletionFlags.Aborted;
-    private bool IsCompleted => (_completionState & StreamCompletionFlags.Completed) == StreamCompletionFlags.Completed;
+    public bool IsAborted => (_completionState & StreamCompletionFlags.Aborted) == StreamCompletionFlags.Aborted;
+    public bool IsCompleted => (_completionState & StreamCompletionFlags.Completed) == StreamCompletionFlags.Completed;
 
     public Pipe RequestBodyPipe { get; private set; } = default!;
     public long? InputRemaining { get; internal set; }
@@ -1232,16 +1232,6 @@ private enum PseudoHeaderFields
         Unknown = 0x40000000
     }
 
-    [Flags]
-    private enum StreamCompletionFlags
-    {
-        None = 0,
-        EndStreamReceived = 1,
-        AbortedRead = 2,
-        Aborted = 4,
-        Completed = 8,
-    }
-
     private static class GracefulCloseInitiator
     {
         public const int None = 0;

src/Servers/Kestrel/Core/src/Internal/Http3/IHttp3Stream.cs

@@ -36,6 +36,10 @@ internal interface IHttp3Stream
 
     bool IsRequestStream { get; }
 
+    bool EndStreamReceived { get; }
+    bool IsAborted { get; }
+    bool IsCompleted { get; }
+
     string TraceIdentifier { get; }
 
     void Abort(ConnectionAbortedException abortReason, Http3ErrorCode errorCode);

src/Servers/Kestrel/Core/src/Internal/Http3/StreamCompletionFlags.cs

@@ -0,0 +1,14 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http3;
+
+[Flags]
+internal enum StreamCompletionFlags
+{
+    None = 0,
+    EndStreamReceived = 1,
+    AbortedRead = 2,
+    Aborted = 4,
+    Completed = 8,
+}

src/Servers/Kestrel/Transport.Quic/src/Internal/QuicLog.cs

@@ -120,7 +120,7 @@ public static void StreamShutdownWrite(ILogger logger, QuicStreamContext streamC
         }
     }
 
-    [LoggerMessage(11, LogLevel.Debug, @"Stream id ""{ConnectionId}"" read aborted by peer with error code {ErrorCode}.", EventName = "StreamAborted", SkipEnabledCheck = true)]
+    [LoggerMessage(11, LogLevel.Debug, @"Stream id ""{ConnectionId}"" read aborted by peer with error code {ErrorCode}.", EventName = "StreamAbortedRead", SkipEnabledCheck = true)]
     private static partial void StreamAbortedReadCore(ILogger logger, string connectionId, long errorCode);
 
     public static void StreamAbortedRead(ILogger logger, QuicStreamContext streamContext, long errorCode)
@@ -131,7 +131,7 @@ public static void StreamAbortedRead(ILogger logger, QuicStreamContext streamCon
         }
     }
 
-    [LoggerMessage(12, LogLevel.Debug, @"Stream id ""{ConnectionId}"" write aborted by peer with error code {ErrorCode}.", EventName = "StreamAborted", SkipEnabledCheck = true)]
+    [LoggerMessage(12, LogLevel.Debug, @"Stream id ""{ConnectionId}"" write aborted by peer with error code {ErrorCode}.", EventName = "StreamAbortedWrite", SkipEnabledCheck = true)]
     private static partial void StreamAbortedWriteCore(ILogger logger, string connectionId, long errorCode);
 
     public static void StreamAbortedWrite(ILogger logger, QuicStreamContext streamContext, long errorCode)
@@ -210,6 +210,28 @@ public static void StreamReused(ILogger logger, QuicStreamContext streamContext)
     [LoggerMessage(21, LogLevel.Debug, "QUIC listener aborted.", EventName = "ConnectionListenerAborted")]
     public static partial void ConnectionListenerAborted(ILogger logger, Exception exception);
 
+    [LoggerMessage(22, LogLevel.Debug, @"Stream id ""{ConnectionId}"" read timed out.", EventName = "StreamTimeoutRead", SkipEnabledCheck = true)]
+    private static partial void StreamTimeoutReadCore(ILogger logger, string connectionId);
+
+    public static void StreamTimeoutRead(ILogger logger, QuicStreamContext streamContext)
+    {
+        if (logger.IsEnabled(LogLevel.Debug))
+        {
+            StreamTimeoutReadCore(logger, streamContext.ConnectionId);
+        }
+    }
+
+    [LoggerMessage(23, LogLevel.Debug, @"Stream id ""{ConnectionId}"" write timed out.", EventName = "StreamTimeoutWrite", SkipEnabledCheck = true)]
+    private static partial void StreamTimeoutWriteCore(ILogger logger, string connectionId);
+
+    public static void StreamTimeoutWrite(ILogger logger, QuicStreamContext streamContext)
+    {
+        if (logger.IsEnabled(LogLevel.Debug))
+        {
+            StreamTimeoutWriteCore(logger, streamContext.ConnectionId);
+        }
+    }
+
     private static StreamType GetStreamType(QuicStreamContext streamContext) =>
         streamContext.CanRead && streamContext.CanWrite
             ? StreamType.Bidirectional

src/Servers/Kestrel/Transport.Quic/src/Internal/QuicStreamContext.cs

@@ -270,7 +270,7 @@ private async ValueTask DoReceiveAsync()
                 }
             }
         }
-        catch (QuicException ex) when (ex.QuicError == QuicError.StreamAborted)
+        catch (QuicException ex) when (ex.QuicError is QuicError.StreamAborted or QuicError.ConnectionAborted)
         {
             // Abort from peer.
             _error = ex.ApplicationErrorCode;
@@ -281,18 +281,15 @@ private async ValueTask DoReceiveAsync()
 
             _clientAbort = true;
         }
-        catch (QuicException ex) when (ex.QuicError == QuicError.ConnectionAborted)
+        catch (QuicException ex) when (ex.QuicError is QuicError.ConnectionIdle)
         {
-            // Abort from peer.
-            _error = ex.ApplicationErrorCode;
-            QuicLog.StreamAbortedRead(_log, this, ex.ApplicationErrorCode.GetValueOrDefault());
+            // Abort from timeout.
+            QuicLog.StreamTimeoutRead(_log, this);
 
             // This could be ignored if _shutdownReason is already set.
             error = new ConnectionResetException(ex.Message, ex);
-
-            _clientAbort = true;
         }
-        catch (QuicException ex) when (ex.QuicError == QuicError.OperationAborted)
+        catch (QuicException ex) when (ex.QuicError is QuicError.OperationAborted)
         {
             // AbortRead has been called for the stream.
             error = new ConnectionAbortedException(ex.Message, ex);
@@ -434,7 +431,7 @@ private async ValueTask DoSendAsync()
                 }
             }
         }
-        catch (QuicException ex) when (ex.QuicError == QuicError.StreamAborted)
+        catch (QuicException ex) when (ex.QuicError is QuicError.StreamAborted or QuicError.ConnectionAborted)
         {
             // Abort from peer.
             _error = ex.ApplicationErrorCode;
@@ -445,18 +442,15 @@ private async ValueTask DoSendAsync()
 
             _clientAbort = true;
         }
-        catch (QuicException ex) when (ex.QuicError == QuicError.ConnectionAborted)
+        catch (QuicException ex) when (ex.QuicError is QuicError.ConnectionIdle)
         {
-            // Abort from peer.
-            _error = ex.ApplicationErrorCode;
-            QuicLog.StreamAbortedWrite(_log, this, ex.ApplicationErrorCode.GetValueOrDefault());
+            // Abort from timeout.
+            QuicLog.StreamTimeoutWrite(_log, this);
 
             // This could be ignored if _shutdownReason is already set.
             shutdownReason = new ConnectionResetException(ex.Message, ex);
-
-            _clientAbort = true;
         }
-        catch (QuicException ex) when (ex.QuicError == QuicError.OperationAborted)
+        catch (QuicException ex) when (ex.QuicError is QuicError.OperationAborted)
         {
             // AbortWrite has been called for the stream.
             // Possibily might also get here from connection closing.