Skip to content

Commit 6887c94

Browse files
JamesNKJamesNK
authored
HTTP/3: Support QUIC idle timeout more cleanly (#43037)
1 parent 3f25b19 commit 6887c94

File tree

12 files changed

+192
-70
lines changed

12 files changed

+192
-70
lines changed

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -713,4 +713,7 @@ For more information on configuring HTTPS see https://go.microsoft.com/fwlink/?l
713713
<data name="NonzeroContentLengthNotAllowedOn205" xml:space="preserve">
714714
<value>Responses with status code 205 cannot have a non-zero Content-Length value.</value>
715715
</data>
716+
<data name="Http3ErrorControlStreamClosed" xml:space="preserve">
717+
<value>A control stream used by the connection was closed or reset.</value>
718+
</data>
716719
</root>

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

Lines changed: 47 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -198,9 +198,49 @@ public void Tick(DateTimeOffset now)
198198
return;
199199
}
200200

201+
ValidateOpenControlStreams(now);
201202
UpdateStreamTimeouts(now);
202203
}
203204

205+
private void ValidateOpenControlStreams(DateTimeOffset now)
206+
{
207+
var ticks = now.Ticks;
208+
209+
// This method validates that a connnection's control streams are open.
210+
//
211+
// They're checked on a delayed timer because when a connection is aborted or timed out, notifications are sent to open streams
212+
// and the connection simultaneously. This is a problem because when a control stream is closed the connection should be aborted
213+
// with the H3_CLOSED_CRITICAL_STREAM status. There is a race between the connection closing for the real reason, and control
214+
// streams closing the connection with H3_CLOSED_CRITICAL_STREAM.
215+
//
216+
// Realistically, control streams are never closed except when the connection is. A small delay in aborting the connection in the
217+
// unlikely situation where a control stream is incorrectly closed should be fine.
218+
ValidateOpenControlStream(OutboundControlStream, this, ticks);
219+
ValidateOpenControlStream(ControlStream, this, ticks);
220+
ValidateOpenControlStream(EncoderStream, this, ticks);
221+
ValidateOpenControlStream(DecoderStream, this, ticks);
222+
223+
static void ValidateOpenControlStream(Http3ControlStream? stream, Http3Connection connection, long ticks)
224+
{
225+
if (stream != null)
226+
{
227+
if (stream.IsCompleted || stream.IsAborted || stream.EndStreamReceived)
228+
{
229+
// If a control stream is no longer active then set a timeout so that the connection is aborted next tick.
230+
if (stream.StreamTimeoutTicks == default)
231+
{
232+
stream.StreamTimeoutTicks = ticks;
233+
}
234+
235+
if (stream.StreamTimeoutTicks < ticks)
236+
{
237+
connection.OnStreamConnectionError(new Http3ConnectionErrorException("A control stream used by the connection was closed or reset.", Http3ErrorCode.ClosedCriticalStream));
238+
}
239+
}
240+
}
241+
}
242+
}
243+
204244
private void UpdateStreamTimeouts(DateTimeOffset now)
205245
{
206246
// This method checks for timeouts:
@@ -271,7 +311,7 @@ private void UpdateStreamTimeouts(DateTimeOffset now)
271311
{
272312
// Cancel connection to be consistent with other data rate limits.
273313
Log.ResponseMinimumDataRateNotSatisfied(_context.ConnectionId, stream.TraceIdentifier);
274-
Abort(new ConnectionAbortedException(CoreStrings.ConnectionTimedBecauseResponseMininumDataRateNotSatisfied), Http3ErrorCode.InternalError);
314+
OnStreamConnectionError(new Http3ConnectionErrorException(CoreStrings.ConnectionTimedBecauseResponseMininumDataRateNotSatisfied, Http3ErrorCode.InternalError));
275315
}
276316
}
277317
}
@@ -651,8 +691,7 @@ private async ValueTask ProcessOutboundControlStreamAsync(Http3ControlStream con
651691
{
652692
try
653693
{
654-
await controlStream.SendStreamIdAsync(id: 0);
655-
await controlStream.SendSettingsFrameAsync();
694+
await controlStream.ProcessOutboundSendsAsync(id: 0);
656695
}
657696
catch (Exception ex)
658697
{
@@ -778,6 +817,11 @@ void IHttp3StreamLifetimeHandler.OnStreamCompleted(IHttp3Stream stream)
778817
}
779818

780819
void IHttp3StreamLifetimeHandler.OnStreamConnectionError(Http3ConnectionErrorException ex)
820+
{
821+
OnStreamConnectionError(ex);
822+
}
823+
824+
private void OnStreamConnectionError(Http3ConnectionErrorException ex)
781825
{
782826
Log.Http3ConnectionError(ConnectionId, ex);
783827
Abort(new ConnectionAbortedException(ex.Message, ex), ex.ErrorCode);

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

Lines changed: 45 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,14 @@ internal abstract class Http3ControlStream : IHttp3Stream, IThreadPoolWorkItem
2929
private volatile int _isClosed;
3030
private long _headerType;
3131
private int _gracefulCloseInitiator;
32-
private bool _connectionClosed;
32+
private readonly object _completionLock = new();
3333

3434
private bool _haveReceivedSettingsFrame;
35+
private StreamCompletionFlags _completionState;
36+
37+
public bool EndStreamReceived => (_completionState & StreamCompletionFlags.EndStreamReceived) == StreamCompletionFlags.EndStreamReceived;
38+
public bool IsAborted => (_completionState & StreamCompletionFlags.Aborted) == StreamCompletionFlags.Aborted;
39+
public bool IsCompleted => (_completionState & StreamCompletionFlags.Completed) == StreamCompletionFlags.Completed;
3540

3641
public long StreamId => _streamIdFeature.StreamId;
3742

@@ -59,8 +64,7 @@ public Http3ControlStream(Http3StreamContext context, long? headerType)
5964

6065
private void OnStreamClosed()
6166
{
62-
Abort(new ConnectionAbortedException("HTTP_CLOSED_CRITICAL_STREAM"), Http3ErrorCode.InternalError);
63-
_connectionClosed = true;
67+
ApplyCompletionFlag(StreamCompletionFlags.Completed);
6468
}
6569

6670
public PipeReader Input => _context.Transport.Input;
@@ -74,15 +78,27 @@ private void OnStreamClosed()
7478

7579
public void Abort(ConnectionAbortedException abortReason, Http3ErrorCode errorCode)
7680
{
77-
// TODO - Should there be a check here to track abort state to avoid
78-
// running twice for a request?
81+
lock (_completionLock)
82+
{
83+
if (IsCompleted || IsAborted)
84+
{
85+
return;
86+
}
87+
88+
var (oldState, newState) = ApplyCompletionFlag(StreamCompletionFlags.Aborted);
89+
90+
if (oldState == newState)
91+
{
92+
return;
93+
}
7994

80-
Log.Http3StreamAbort(_context.ConnectionId, errorCode, abortReason);
95+
Log.Http3StreamAbort(_context.ConnectionId, errorCode, abortReason);
8196

82-
_errorCodeFeature.Error = (long)errorCode;
83-
_frameWriter.Abort(abortReason);
97+
_errorCodeFeature.Error = (long)errorCode;
98+
_frameWriter.Abort(abortReason);
8499

85-
Input.Complete(abortReason);
100+
Input.Complete(abortReason);
101+
}
86102
}
87103

88104
public void OnInputOrOutputCompleted()
@@ -101,9 +117,27 @@ private bool TryClose()
101117
return false;
102118
}
103119

104-
internal async ValueTask SendStreamIdAsync(long id)
120+
private (StreamCompletionFlags OldState, StreamCompletionFlags NewState) ApplyCompletionFlag(StreamCompletionFlags completionState)
121+
{
122+
lock (_completionLock)
123+
{
124+
var oldCompletionState = _completionState;
125+
_completionState |= completionState;
126+
127+
return (oldCompletionState, _completionState);
128+
}
129+
}
130+
131+
internal async ValueTask ProcessOutboundSendsAsync(long id)
105132
{
133+
_streamClosedFeature.OnClosed(static state =>
134+
{
135+
var stream = (Http3ControlStream)state!;
136+
stream.OnStreamClosed();
137+
}, this);
138+
106139
await _frameWriter.WriteStreamIdAsync(id);
140+
await _frameWriter.WriteSettingsAsync(_serverPeerSettings.GetNonProtocolDefaults());
107141
}
108142

109143
internal ValueTask<FlushResult> SendGoAway(long id)
@@ -112,11 +146,6 @@ internal ValueTask<FlushResult> SendGoAway(long id)
112146
return _frameWriter.WriteGoAway(id);
113147
}
114148

115-
internal async ValueTask SendSettingsFrameAsync()
116-
{
117-
await _frameWriter.WriteSettingsAsync(_serverPeerSettings.GetNonProtocolDefaults());
118-
}
119-
120149
private async ValueTask<long> TryReadStreamHeaderAsync()
121150
{
122151
// 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
212241
}
213242
finally
214243
{
244+
ApplyCompletionFlag(StreamCompletionFlags.Completed);
215245
_context.StreamLifetimeHandler.OnStreamCompleted(this);
216246
}
217247
}
@@ -241,12 +271,6 @@ private async Task HandleControlStream()
241271

242272
if (result.IsCompleted)
243273
{
244-
if (!_connectionClosed)
245-
{
246-
// https://quicwg.org/base-drafts/draft-ietf-quic-http.html#section-6.2.1-2
247-
throw new Http3ConnectionErrorException(CoreStrings.Http3ErrorControlStreamClientClosedInbound, Http3ErrorCode.ClosedCriticalStream);
248-
}
249-
250274
return;
251275
}
252276
}

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

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,8 @@ internal abstract partial class Http3Stream : HttpProtocol, IHttp3Stream, IHttpS
6262
protected readonly Http3RawFrame _incomingFrame = new();
6363

6464
public bool EndStreamReceived => (_completionState & StreamCompletionFlags.EndStreamReceived) == StreamCompletionFlags.EndStreamReceived;
65-
private bool IsAborted => (_completionState & StreamCompletionFlags.Aborted) == StreamCompletionFlags.Aborted;
66-
private bool IsCompleted => (_completionState & StreamCompletionFlags.Completed) == StreamCompletionFlags.Completed;
65+
public bool IsAborted => (_completionState & StreamCompletionFlags.Aborted) == StreamCompletionFlags.Aborted;
66+
public bool IsCompleted => (_completionState & StreamCompletionFlags.Completed) == StreamCompletionFlags.Completed;
6767

6868
public Pipe RequestBodyPipe { get; private set; } = default!;
6969
public long? InputRemaining { get; internal set; }
@@ -1232,16 +1232,6 @@ private enum PseudoHeaderFields
12321232
Unknown = 0x40000000
12331233
}
12341234

1235-
[Flags]
1236-
private enum StreamCompletionFlags
1237-
{
1238-
None = 0,
1239-
EndStreamReceived = 1,
1240-
AbortedRead = 2,
1241-
Aborted = 4,
1242-
Completed = 8,
1243-
}
1244-
12451235
private static class GracefulCloseInitiator
12461236
{
12471237
public const int None = 0;

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,10 @@ internal interface IHttp3Stream
3636

3737
bool IsRequestStream { get; }
3838

39+
bool EndStreamReceived { get; }
40+
bool IsAborted { get; }
41+
bool IsCompleted { get; }
42+
3943
string TraceIdentifier { get; }
4044

4145
void Abort(ConnectionAbortedException abortReason, Http3ErrorCode errorCode);

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http3;
5+
6+
[Flags]
7+
internal enum StreamCompletionFlags
8+
{
9+
None = 0,
10+
EndStreamReceived = 1,
11+
AbortedRead = 2,
12+
Aborted = 4,
13+
Completed = 8,
14+
}

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

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ public static void StreamShutdownWrite(ILogger logger, QuicStreamContext streamC
120120
}
121121
}
122122

123-
[LoggerMessage(11, LogLevel.Debug, @"Stream id ""{ConnectionId}"" read aborted by peer with error code {ErrorCode}.", EventName = "StreamAborted", SkipEnabledCheck = true)]
123+
[LoggerMessage(11, LogLevel.Debug, @"Stream id ""{ConnectionId}"" read aborted by peer with error code {ErrorCode}.", EventName = "StreamAbortedRead", SkipEnabledCheck = true)]
124124
private static partial void StreamAbortedReadCore(ILogger logger, string connectionId, long errorCode);
125125

126126
public static void StreamAbortedRead(ILogger logger, QuicStreamContext streamContext, long errorCode)
@@ -131,7 +131,7 @@ public static void StreamAbortedRead(ILogger logger, QuicStreamContext streamCon
131131
}
132132
}
133133

134-
[LoggerMessage(12, LogLevel.Debug, @"Stream id ""{ConnectionId}"" write aborted by peer with error code {ErrorCode}.", EventName = "StreamAborted", SkipEnabledCheck = true)]
134+
[LoggerMessage(12, LogLevel.Debug, @"Stream id ""{ConnectionId}"" write aborted by peer with error code {ErrorCode}.", EventName = "StreamAbortedWrite", SkipEnabledCheck = true)]
135135
private static partial void StreamAbortedWriteCore(ILogger logger, string connectionId, long errorCode);
136136

137137
public static void StreamAbortedWrite(ILogger logger, QuicStreamContext streamContext, long errorCode)
@@ -210,6 +210,28 @@ public static void StreamReused(ILogger logger, QuicStreamContext streamContext)
210210
[LoggerMessage(21, LogLevel.Debug, "QUIC listener aborted.", EventName = "ConnectionListenerAborted")]
211211
public static partial void ConnectionListenerAborted(ILogger logger, Exception exception);
212212

213+
[LoggerMessage(22, LogLevel.Debug, @"Stream id ""{ConnectionId}"" read timed out.", EventName = "StreamTimeoutRead", SkipEnabledCheck = true)]
214+
private static partial void StreamTimeoutReadCore(ILogger logger, string connectionId);
215+
216+
public static void StreamTimeoutRead(ILogger logger, QuicStreamContext streamContext)
217+
{
218+
if (logger.IsEnabled(LogLevel.Debug))
219+
{
220+
StreamTimeoutReadCore(logger, streamContext.ConnectionId);
221+
}
222+
}
223+
224+
[LoggerMessage(23, LogLevel.Debug, @"Stream id ""{ConnectionId}"" write timed out.", EventName = "StreamTimeoutWrite", SkipEnabledCheck = true)]
225+
private static partial void StreamTimeoutWriteCore(ILogger logger, string connectionId);
226+
227+
public static void StreamTimeoutWrite(ILogger logger, QuicStreamContext streamContext)
228+
{
229+
if (logger.IsEnabled(LogLevel.Debug))
230+
{
231+
StreamTimeoutWriteCore(logger, streamContext.ConnectionId);
232+
}
233+
}
234+
213235
private static StreamType GetStreamType(QuicStreamContext streamContext) =>
214236
streamContext.CanRead && streamContext.CanWrite
215237
? StreamType.Bidirectional

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

Lines changed: 10 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -270,7 +270,7 @@ private async ValueTask DoReceiveAsync()
270270
}
271271
}
272272
}
273-
catch (QuicException ex) when (ex.QuicError == QuicError.StreamAborted)
273+
catch (QuicException ex) when (ex.QuicError is QuicError.StreamAborted or QuicError.ConnectionAborted)
274274
{
275275
// Abort from peer.
276276
_error = ex.ApplicationErrorCode;
@@ -281,18 +281,15 @@ private async ValueTask DoReceiveAsync()
281281

282282
_clientAbort = true;
283283
}
284-
catch (QuicException ex) when (ex.QuicError == QuicError.ConnectionAborted)
284+
catch (QuicException ex) when (ex.QuicError is QuicError.ConnectionIdle)
285285
{
286-
// Abort from peer.
287-
_error = ex.ApplicationErrorCode;
288-
QuicLog.StreamAbortedRead(_log, this, ex.ApplicationErrorCode.GetValueOrDefault());
286+
// Abort from timeout.
287+
QuicLog.StreamTimeoutRead(_log, this);
289288

290289
// This could be ignored if _shutdownReason is already set.
291290
error = new ConnectionResetException(ex.Message, ex);
292-
293-
_clientAbort = true;
294291
}
295-
catch (QuicException ex) when (ex.QuicError == QuicError.OperationAborted)
292+
catch (QuicException ex) when (ex.QuicError is QuicError.OperationAborted)
296293
{
297294
// AbortRead has been called for the stream.
298295
error = new ConnectionAbortedException(ex.Message, ex);
@@ -434,7 +431,7 @@ private async ValueTask DoSendAsync()
434431
}
435432
}
436433
}
437-
catch (QuicException ex) when (ex.QuicError == QuicError.StreamAborted)
434+
catch (QuicException ex) when (ex.QuicError is QuicError.StreamAborted or QuicError.ConnectionAborted)
438435
{
439436
// Abort from peer.
440437
_error = ex.ApplicationErrorCode;
@@ -445,18 +442,15 @@ private async ValueTask DoSendAsync()
445442

446443
_clientAbort = true;
447444
}
448-
catch (QuicException ex) when (ex.QuicError == QuicError.ConnectionAborted)
445+
catch (QuicException ex) when (ex.QuicError is QuicError.ConnectionIdle)
449446
{
450-
// Abort from peer.
451-
_error = ex.ApplicationErrorCode;
452-
QuicLog.StreamAbortedWrite(_log, this, ex.ApplicationErrorCode.GetValueOrDefault());
447+
// Abort from timeout.
448+
QuicLog.StreamTimeoutWrite(_log, this);
453449

454450
// This could be ignored if _shutdownReason is already set.
455451
shutdownReason = new ConnectionResetException(ex.Message, ex);
456-
457-
_clientAbort = true;
458452
}
459-
catch (QuicException ex) when (ex.QuicError == QuicError.OperationAborted)
453+
catch (QuicException ex) when (ex.QuicError is QuicError.OperationAborted)
460454
{
461455
// AbortWrite has been called for the stream.
462456
// Possibily might also get here from connection closing.

0 commit comments

Comments
 (0)