Skip to content

Commit dae4b3d

Browse files
jkotalikjkotalik
committed
Feedback
1 parent fd2cae5 commit dae4b3d

25 files changed

+116
-555
lines changed

‎src/Servers/Connections.Abstractions/src/BaseConnectionContext.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

4+
using System;
45
using System.Collections.Generic;
56
using System.Net;
67
using System.Threading;
@@ -9,7 +10,7 @@
910

1011
namespace Microsoft.AspNetCore.Connections
1112
{
12-
public abstract class BaseConnectionContext
13+
public abstract class BaseConnectionContext : IAsyncDisposable
1314
{
1415
/// <summary>
1516
/// Gets or sets a unique identifier to represent this connection in trace logs.

‎src/Servers/Connections.Abstractions/src/MultiplexedConnectionContext.cs‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,21 +8,24 @@
88

99
namespace Microsoft.AspNetCore.Connections
1010
{
11+
/// <summary>
12+
/// Encapsulates all information about a multiplexed connection.
13+
/// </summary>
1114
public abstract class MultiplexedConnectionContext : BaseConnectionContext, IAsyncDisposable
1215
{
1316
/// <summary>
1417
/// Asynchronously accept an incoming stream on the connection.
1518
/// </summary>
1619
/// <param name="cancellationToken"></param>
1720
/// <returns></returns>
18-
public abstract ValueTask<StreamContext> AcceptAsync(CancellationToken cancellationToken = default);
21+
public abstract ValueTask<ConnectionContext> AcceptAsync(CancellationToken cancellationToken = default);
1922

2023
/// <summary>
2124
/// Creates an outbound connection
2225
/// </summary>
2326
/// <param name="features"></param>
2427
/// <param name="cancellationToken"></param>
2528
/// <returns></returns>
26-
public abstract ValueTask<StreamContext> ConnectAsync(IFeatureCollection features = null, CancellationToken cancellationToken = default);
29+
public abstract ValueTask<ConnectionContext> ConnectAsync(IFeatureCollection features = null, CancellationToken cancellationToken = default);
2730
}
2831
}

‎src/Servers/Connections.Abstractions/src/StreamContext.cs‎

Lines changed: 0 additions & 13 deletions
This file was deleted.

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

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
using System;
55
using System.Collections.Concurrent;
6+
using System.Collections.Generic;
67
using System.Diagnostics;
78
using System.Net;
89
using System.Net.Http;
@@ -26,7 +27,7 @@ internal class Http3Connection : IRequestProcessor, ITimeoutHandler
2627
public Http3ControlStream EncoderStream { get; set; }
2728
public Http3ControlStream DecoderStream { get; set; }
2829

29-
private readonly ConcurrentDictionary<long, Http3Stream> _streams = new ConcurrentDictionary<long, Http3Stream>();
30+
internal readonly Dictionary<long, Http3Stream> _streams = new Dictionary<long, Http3Stream>();
3031

3132
private long _highestOpenedStreamId; // TODO lock to access
3233
private volatile bool _haveSentGoAway;
@@ -212,7 +213,7 @@ internal async Task InnerProcessRequestsAsync<TContext>(IHttpApplication<TContex
212213

213214
var httpConnectionContext = new Http3StreamContext
214215
{
215-
ConnectionId = streamContext.StreamId,
216+
ConnectionId = streamContext.ConnectionId,
216217
StreamContext = streamContext,
217218
// TODO connection context is null here. Should we set it to anything?
218219
ServiceContext = _context.ServiceContext,
@@ -238,17 +239,23 @@ internal async Task InnerProcessRequestsAsync<TContext>(IHttpApplication<TContex
238239

239240
var http3Stream = new Http3Stream<TContext>(application, this, httpConnectionContext);
240241
var stream = http3Stream;
241-
_streams[streamId] = http3Stream;
242+
lock (_streams)
243+
{
244+
_streams[streamId] = http3Stream;
245+
}
242246
ThreadPool.UnsafeQueueUserWorkItem(stream, preferLocal: false);
243247
}
244248
}
245249
}
246250
finally
247251
{
248252
// Abort all streams as connection has shutdown.
249-
foreach (var stream in _streams.Values)
253+
lock (_streams)
250254
{
251-
stream.Abort(new ConnectionAbortedException("Connection is shutting down."));
255+
foreach (var stream in _streams.Values)
256+
{
257+
stream.Abort(new ConnectionAbortedException("Connection is shutting down."));
258+
}
252259
}
253260

254261
ControlStream?.Abort(new ConnectionAbortedException("Connection is shutting down."));
@@ -335,10 +342,14 @@ private void InnerAbort(ConnectionAbortedException ex)
335342
_haveSentGoAway = true;
336343

337344
// Abort currently active streams
338-
foreach (var stream in _streams.Values)
345+
lock (_streams)
339346
{
340-
stream.Abort(new ConnectionAbortedException("The Http3Connection has been aborted"), Http3ErrorCode.UnexpectedFrame);
347+
foreach (var stream in _streams.Values)
348+
{
349+
stream.Abort(new ConnectionAbortedException("The Http3Connection has been aborted"), Http3ErrorCode.UnexpectedFrame);
350+
}
341351
}
352+
342353
// TODO need to figure out if there is server initiated connection close rather than stream close?
343354
}
344355

@@ -356,5 +367,13 @@ internal void ApplyMaxTableCapacity(long value)
356367
// TODO make sure this works
357368
//_maxDynamicTableSize = value;
358369
}
370+
371+
internal void RemoveStream(long streamId)
372+
{
373+
lock(_streams)
374+
{
375+
_streams.Remove(streamId);
376+
}
377+
}
359378
}
360379
}

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

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ internal abstract class Http3Stream : HttpProtocol, IHttpHeadersHandler, IThread
3838
private int _gracefulCloseInitiator;
3939
private readonly Http3StreamContext _context;
4040
private readonly IProtocolErrorCodeFeature _errorCodeFeature;
41+
private readonly IStreamIdFeature _streamIdFeature;
4142
private readonly Http3RawFrame _incomingFrame = new Http3RawFrame();
4243
private RequestHeaderParsingState _requestHeaderParsingState;
4344
private PseudoHeaderFields _parsedPseudoHeaderFields;
@@ -62,6 +63,7 @@ public Http3Stream(Http3Connection http3Connection, Http3StreamContext context)
6263
_context = context;
6364

6465
_errorCodeFeature = _context.ConnectionFeatures.Get<IProtocolErrorCodeFeature>();
66+
_streamIdFeature = _context.ConnectionFeatures.Get<IStreamIdFeature>();
6567

6668
_frameWriter = new Http3FrameWriter(
6769
context.Transport.Output,
@@ -297,7 +299,6 @@ public void HandleRequestHeadersTimeout()
297299

298300
public void OnInputOrOutputCompleted()
299301
{
300-
Log.LogTrace("On input or output completed");
301302
TryClose();
302303
Abort(new ConnectionAbortedException(CoreStrings.ConnectionAbortedByClient), Http3ErrorCode.NoError);
303304
}
@@ -353,7 +354,6 @@ public async Task ProcessRequestAsync<TContext>(IHttpApplication<TContext> appli
353354
catch (Http3StreamErrorException ex)
354355
{
355356
error = ex;
356-
//errorCode = ex.ErrorCode;
357357
Abort(new ConnectionAbortedException(ex.Message, ex), ex.ErrorCode);
358358
}
359359
catch (Exception ex)
@@ -366,10 +366,8 @@ public async Task ProcessRequestAsync<TContext>(IHttpApplication<TContext> appli
366366
var streamError = error as ConnectionAbortedException
367367
?? new ConnectionAbortedException("The stream has completed.", error);
368368

369-
// Input has completed.
370-
371369
Input.Complete();
372-
_context.Transport.Input.CancelPendingRead();
370+
373371
await RequestBodyPipe.Writer.CompleteAsync();
374372

375373
// Make sure application func is completed before completing writer.
@@ -386,6 +384,7 @@ public async Task ProcessRequestAsync<TContext>(IHttpApplication<TContext> appli
386384
}
387385
finally
388386
{
387+
_http3Connection.RemoveStream(_streamIdFeature.StreamId);
389388
}
390389
}
391390
}
@@ -727,7 +726,6 @@ private enum RequestHeaderParsingState
727726
Trailers
728727
}
729728

730-
731729
[Flags]
732730
private enum PseudoHeaderFields
733731
{

‎src/Servers/Kestrel/Core/src/Internal/Http3StreamContext.cs‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,12 @@
11
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

4-
using System.Buffers;
5-
using System.IO.Pipelines;
6-
using System.Net;
74
using Microsoft.AspNetCore.Connections;
8-
using Microsoft.AspNetCore.Http.Features;
9-
using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure;
105

116
namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal
127
{
138
internal class Http3StreamContext : HttpConnectionContext
149
{
15-
public StreamContext StreamContext { get; set; }
10+
public ConnectionContext StreamContext { get; set; }
1611
}
1712
}

‎src/Servers/Kestrel/Core/src/Internal/MultiplexedConnectionDispatcher.cs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,6 @@ async Task AcceptConnectionsAsync()
5353

5454
// Add the connection to the connection manager before we queue it for execution
5555
var id = Interlocked.Increment(ref _lastConnectionId);
56-
// TODO Don't pass null in here! use a base class
5756
var kestrelConnection = new KestrelConnection<MultiplexedConnectionContext>(id, _serviceContext, c => _connectionDelegate(c), connection, Log);
5857

5958
_serviceContext.ConnectionManager.AddConnection(id, kestrelConnection);

‎src/Servers/Kestrel/Core/src/KestrelServer.cs‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ async Task OnBind(ListenOptions options)
148148
// sockets for it to successfully listen. It also seems racy.
149149
if ((options.Protocols & HttpProtocols.Http3) == HttpProtocols.Http3)
150150
{
151-
if (_multiplexedTransportFactories == null)
151+
if (_multiplexedTransportFactories == null || _multiplexedTransportFactories.Count == 0)
152152
{
153153
throw new InvalidOperationException("Cannot start HTTP/3 server if no MultiplexedTransportFactories are registered.");
154154
}
@@ -213,19 +213,19 @@ public async Task StopAsync(CancellationToken cancellationToken)
213213

214214
try
215215
{
216-
var transportCount = _transports.Count;
216+
var connectionTransportCount = _transports.Count;
217217
var totalTransportCount = _transports.Count + _multiplexedTransports.Count;
218218
var tasks = new Task[totalTransportCount];
219219

220-
for (int i = 0; i < transportCount; i++)
220+
for (int i = 0; i < connectionTransportCount; i++)
221221
{
222222
(IConnectionListener listener, Task acceptLoop) = _transports[i];
223223
tasks[i] = Task.WhenAll(listener.UnbindAsync(cancellationToken).AsTask(), acceptLoop);
224224
}
225225

226-
for (int i = transportCount; i < totalTransportCount; i++)
226+
for (int i = connectionTransportCount; i < totalTransportCount; i++)
227227
{
228-
(IMultiplexedConnectionListener listener, Task acceptLoop) = _multiplexedTransports[i - transportCount];
228+
(IMultiplexedConnectionListener listener, Task acceptLoop) = _multiplexedTransports[i - connectionTransportCount];
229229
tasks[i] = Task.WhenAll(listener.UnbindAsync(cancellationToken).AsTask(), acceptLoop);
230230
}
231231

@@ -241,15 +241,15 @@ public async Task StopAsync(CancellationToken cancellationToken)
241241
}
242242
}
243243

244-
for (int i = 0; i < _transports.Count; i++)
244+
for (int i = 0; i < connectionTransportCount; i++)
245245
{
246246
(IConnectionListener listener, Task acceptLoop) = _transports[i];
247247
tasks[i] = listener.DisposeAsync().AsTask();
248248
}
249249

250-
for (int i = _transports.Count; i < totalTransportCount; i++)
250+
for (int i = connectionTransportCount; i < totalTransportCount; i++)
251251
{
252-
(IMultiplexedConnectionListener listener, Task acceptLoop) = _multiplexedTransports[i - transportCount];
252+
(IMultiplexedConnectionListener listener, Task acceptLoop) = _multiplexedTransports[i - connectionTransportCount];
253253
tasks[i] = listener.DisposeAsync().AsTask();
254254
}
255255

‎src/Servers/Kestrel/Kestrel/test/GeneratedCodeTests.cs‎

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,17 +20,15 @@ public void GeneratedCodeIsUpToDate()
2020
var httpProtocolGeneratedPath = Path.Combine(AppContext.BaseDirectory, "shared", "GeneratedContent", "HttpProtocol.Generated.cs");
2121
var httpUtilitiesGeneratedPath = Path.Combine(AppContext.BaseDirectory, "shared", "GeneratedContent", "HttpUtilities.Generated.cs");
2222
var http2ConnectionGeneratedPath = Path.Combine(AppContext.BaseDirectory, "shared", "GeneratedContent", "Http2Connection.Generated.cs");
23-
var transportMultiplexedConnectionGeneratedPath = Path.Combine(AppContext.BaseDirectory,"shared", "GeneratedContent", "TransportConnectionBase.Generated.cs");
23+
var transportMultiplexedConnectionGeneratedPath = Path.Combine(AppContext.BaseDirectory,"shared", "GeneratedContent", "TransportMultiplexedConnection.Generated.cs");
2424
var transportConnectionGeneratedPath = Path.Combine(AppContext.BaseDirectory,"shared", "GeneratedContent", "TransportConnection.Generated.cs");
25-
var transportStreamGeneratedPath = Path.Combine(AppContext.BaseDirectory,"shared", "GeneratedContent", "TransportStream.Generated.cs");
2625

2726
var testHttpHeadersGeneratedPath = Path.GetTempFileName();
2827
var testHttpProtocolGeneratedPath = Path.GetTempFileName();
2928
var testHttpUtilitiesGeneratedPath = Path.GetTempFileName();
3029
var testHttp2ConnectionGeneratedPath = Path.GetTempFileName();
3130
var testTransportMultiplexedConnectionGeneratedPath = Path.GetTempFileName();
3231
var testTransportConnectionGeneratedPath = Path.GetTempFileName();
33-
var testTransportStreamGeneratedPath = Path.GetTempFileName();
3432

3533
try
3634
{
@@ -40,31 +38,27 @@ public void GeneratedCodeIsUpToDate()
4038
var currentHttp2ConnectionGenerated = File.ReadAllText(http2ConnectionGeneratedPath);
4139
var currentTransportConnectionBaseGenerated = File.ReadAllText(transportMultiplexedConnectionGeneratedPath);
4240
var currentTransportConnectionGenerated = File.ReadAllText(transportConnectionGeneratedPath);
43-
var currentTransportStreamGenerated = File.ReadAllText(transportStreamGeneratedPath);
4441

4542
CodeGenerator.Program.Run(testHttpHeadersGeneratedPath,
4643
testHttpProtocolGeneratedPath,
4744
testHttpUtilitiesGeneratedPath,
4845
testHttp2ConnectionGeneratedPath,
4946
testTransportMultiplexedConnectionGeneratedPath,
50-
testTransportConnectionGeneratedPath,
51-
testTransportStreamGeneratedPath);
47+
testTransportConnectionGeneratedPath);
5248

5349
var testHttpHeadersGenerated = File.ReadAllText(testHttpHeadersGeneratedPath);
5450
var testHttpProtocolGenerated = File.ReadAllText(testHttpProtocolGeneratedPath);
5551
var testHttpUtilitiesGenerated = File.ReadAllText(testHttpUtilitiesGeneratedPath);
5652
var testHttp2ConnectionGenerated = File.ReadAllText(testHttp2ConnectionGeneratedPath);
5753
var testTransportMultiplxedConnectionGenerated = File.ReadAllText(testTransportMultiplexedConnectionGeneratedPath);
5854
var testTransportConnectionGenerated = File.ReadAllText(testTransportConnectionGeneratedPath);
59-
var testTransportStreamGenerated = File.ReadAllText(testTransportStreamGeneratedPath);
6055

6156
Assert.Equal(currentHttpHeadersGenerated, testHttpHeadersGenerated, ignoreLineEndingDifferences: true);
6257
Assert.Equal(currentHttpProtocolGenerated, testHttpProtocolGenerated, ignoreLineEndingDifferences: true);
6358
Assert.Equal(currentHttpUtilitiesGenerated, testHttpUtilitiesGenerated, ignoreLineEndingDifferences: true);
6459
Assert.Equal(currentHttp2ConnectionGenerated, testHttp2ConnectionGenerated, ignoreLineEndingDifferences: true);
6560
Assert.Equal(currentTransportConnectionBaseGenerated, testTransportMultiplxedConnectionGenerated, ignoreLineEndingDifferences: true);
6661
Assert.Equal(currentTransportConnectionGenerated, testTransportConnectionGenerated, ignoreLineEndingDifferences: true);
67-
Assert.Equal(currentTransportStreamGenerated, testTransportStreamGenerated, ignoreLineEndingDifferences: true);
6862
}
6963
finally
7064
{
@@ -74,7 +68,6 @@ public void GeneratedCodeIsUpToDate()
7468
File.Delete(testHttp2ConnectionGeneratedPath);
7569
File.Delete(testTransportMultiplexedConnectionGeneratedPath);
7670
File.Delete(testTransportConnectionGeneratedPath);
77-
File.Delete(testTransportStreamGeneratedPath);
7871
}
7972
}
8073
}

‎src/Servers/Kestrel/Kestrel/test/Microsoft.AspNetCore.Server.Kestrel.Tests.csproj‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
<Content Include="$(KestrelRoot)Core\src\Internal\Infrastructure\HttpUtilities.Generated.cs" LinkBase="shared\GeneratedContent" CopyToOutputDirectory="PreserveNewest" />
1515
<Content Include="$(KestrelRoot)Core\src\Internal\Http2\Http2Connection.Generated.cs" LinkBase="shared\GeneratedContent" CopyToOutputDirectory="PreserveNewest" />
1616
<Content Include="$(KestrelSharedSourceRoot)\TransportConnection.Generated.cs" LinkBase="shared\GeneratedContent" CopyToOutputDirectory="PreserveNewest" />
17+
<Content Include="$(KestrelSharedSourceRoot)\TransportMultiplexedConnection.Generated.cs" LinkBase="shared\GeneratedContent" CopyToOutputDirectory="PreserveNewest" />
1718
</ItemGroup>
1819

1920
<ItemGroup Condition="'$(TargetFrameworkIdentifier)' != '.NETFramework'">

0 commit comments

Comments
 (0)