PR feedback

JamesNK · JamesNK · commit f97debe6fd4f · 2021-07-17T12:51:46.000+12:00
diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.Generated.cs b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.Generated.cs
@@ -5,6 +5,7 @@
 using System.Collections;
 using System.Collections.Generic;
 using System.Runtime.CompilerServices;
+
 using Microsoft.AspNetCore.Connections.Features;
 using Microsoft.AspNetCore.Http.Features;
 using Microsoft.AspNetCore.Http.Features.Authentication;
diff --git a/src/Servers/Kestrel/Transport.Quic/src/Internal/QuicStreamContext.cs b/src/Servers/Kestrel/Transport.Quic/src/Internal/QuicStreamContext.cs
@@ -132,6 +132,8 @@ public override string ConnectionId
 
         public void Start()
         {
+            Debug.Assert(_processingTask.IsCompletedSuccessfully);
+
             _processingTask = StartAsync();
         }
 
diff --git a/src/Servers/Kestrel/Transport.Quic/test/QuicConnectionContextTests.cs b/src/Servers/Kestrel/Transport.Quic/test/QuicConnectionContextTests.cs
@@ -404,7 +404,7 @@ public async Task StreamPool_ManyConcurrentStreams_StreamPoolFull()
                 // TODO: Race condition in QUIC library.
                 // Delay between sending streams to avoid
                 // https://github.com/dotnet/runtime/issues/55249
-                await Task.Delay(50);
+                await Task.Delay(100);
                 streamTasks.Add(SendStream(requestState));
             }
 
diff --git a/src/Servers/Kestrel/shared/PooledStreamStack.cs b/src/Servers/Kestrel/shared/PooledStreamStack.cs
@@ -14,7 +14,7 @@ internal interface IPooledStream
         void DisposeCore();
     }
 
-    // See https://github.com/dotnet/runtime/blob/master/src/libraries/System.IO.Pipelines/src/System/IO/Pipelines/BufferSegmentStack.cs
+    // See https://github.com/dotnet/runtime/blob/da9b16f2804e87c9c1ca9dcd9036e7b53e724f5d/src/libraries/System.IO.Pipelines/src/System/IO/Pipelines/BufferSegmentStack.cs
     internal struct PooledStreamStack<TValue> where TValue : class, IPooledStream
     {
         // Internal for testing
@@ -139,6 +139,7 @@ private static int CalculateRemoveCount(long now, int size, StreamAsValueType[]
             return size;
         }
 
+        // See https://github.com/dotnet/runtime/blob/da9b16f2804e87c9c1ca9dcd9036e7b53e724f5d/src/libraries/System.IO.Pipelines/src/System/IO/Pipelines/BufferSegmentStack.cs#L68-L79
         internal readonly struct StreamAsValueType
         {
             private readonly TValue _value;
diff --git a/src/Servers/Kestrel/shared/TransportMultiplexedConnection.FeatureCollection.cs b/src/Servers/Kestrel/shared/TransportMultiplexedConnection.FeatureCollection.cs
@@ -16,8 +16,6 @@ internal partial class TransportMultiplexedConnection
         // and the code generator re-reun, which will change the interface list.
         // See also: tools/CodeGenerator/TransportConnectionFeatureCollection.cs
 
-        private IDictionary<object, object?>? _persistentState;
-
         MemoryPool<byte> IMemoryPoolFeature.MemoryPool => MemoryPool;
 
         IDictionary<object, object?> IConnectionItemsFeature.Items
@@ -33,14 +31,5 @@ CancellationToken IConnectionLifetimeFeature.ConnectionClosed
         }
 
         void IConnectionLifetimeFeature.Abort() => Abort(new ConnectionAbortedException("The connection was aborted by the application via IConnectionLifetimeFeature.Abort()."));
-
-        IDictionary<object, object?> IPersistentStateFeature.State
-        {
-            get
-            {
-                // Lazily allocate persistent state
-                return _persistentState ?? (_persistentState = new ConnectionItems());
-            }
-        }
     }
 }
diff --git a/src/Servers/Kestrel/shared/TransportMultiplexedConnection.Generated.cs b/src/Servers/Kestrel/shared/TransportMultiplexedConnection.Generated.cs
@@ -16,14 +16,12 @@ namespace Microsoft.AspNetCore.Connections
     internal partial class TransportMultiplexedConnection : IFeatureCollection,
                                                             IConnectionIdFeature,
                                                             IConnectionItemsFeature,
-                                                            IPersistentStateFeature,
                                                             IMemoryPoolFeature,
                                                             IConnectionLifetimeFeature
     {
         // Implemented features
         internal protected IConnectionIdFeature? _currentIConnectionIdFeature;
         internal protected IConnectionItemsFeature? _currentIConnectionItemsFeature;
-        internal protected IPersistentStateFeature? _currentIPersistentStateFeature;
         internal protected IMemoryPoolFeature? _currentIMemoryPoolFeature;
         internal protected IConnectionLifetimeFeature? _currentIConnectionLifetimeFeature;
 
@@ -38,7 +36,6 @@ private void FastReset()
         {
             _currentIConnectionIdFeature = this;
             _currentIConnectionItemsFeature = this;
-            _currentIPersistentStateFeature = this;
             _currentIMemoryPoolFeature = this;
             _currentIConnectionLifetimeFeature = this;
 
@@ -126,10 +123,6 @@ private void ExtraFeatureSet(Type key, object? value)
                 {
                     feature = _currentIConnectionItemsFeature;
                 }
-                else if (key == typeof(IPersistentStateFeature))
-                {
-                    feature = _currentIPersistentStateFeature;
-                }
                 else if (key == typeof(IMemoryPoolFeature))
                 {
                     feature = _currentIMemoryPoolFeature;
@@ -162,10 +155,6 @@ private void ExtraFeatureSet(Type key, object? value)
                 {
                     _currentIConnectionItemsFeature = (IConnectionItemsFeature?)value;
                 }
-                else if (key == typeof(IPersistentStateFeature))
-                {
-                    _currentIPersistentStateFeature = (IPersistentStateFeature?)value;
-                }
                 else if (key == typeof(IMemoryPoolFeature))
                 {
                     _currentIMemoryPoolFeature = (IMemoryPoolFeature?)value;
@@ -200,10 +189,6 @@ private void ExtraFeatureSet(Type key, object? value)
             {
                 feature = Unsafe.As<IConnectionItemsFeature?, TFeature?>(ref _currentIConnectionItemsFeature);
             }
-            else if (typeof(TFeature) == typeof(IPersistentStateFeature))
-            {
-                feature = Unsafe.As<IPersistentStateFeature?, TFeature?>(ref _currentIPersistentStateFeature);
-            }
             else if (typeof(TFeature) == typeof(IMemoryPoolFeature))
             {
                 feature = Unsafe.As<IMemoryPoolFeature?, TFeature?>(ref _currentIMemoryPoolFeature);
@@ -239,10 +224,6 @@ private void ExtraFeatureSet(Type key, object? value)
             {
                 _currentIConnectionItemsFeature = Unsafe.As<TFeature?, IConnectionItemsFeature?>(ref feature);
             }
-            else if (typeof(TFeature) == typeof(IPersistentStateFeature))
-            {
-                _currentIPersistentStateFeature = Unsafe.As<TFeature?, IPersistentStateFeature?>(ref feature);
-            }
             else if (typeof(TFeature) == typeof(IMemoryPoolFeature))
             {
                 _currentIMemoryPoolFeature = Unsafe.As<TFeature?, IMemoryPoolFeature?>(ref feature);
@@ -271,10 +252,6 @@ private IEnumerable<KeyValuePair<Type, object>> FastEnumerable()
             {
                 yield return new KeyValuePair<Type, object>(typeof(IConnectionItemsFeature), _currentIConnectionItemsFeature);
             }
-            if (_currentIPersistentStateFeature != null)
-            {
-                yield return new KeyValuePair<Type, object>(typeof(IPersistentStateFeature), _currentIPersistentStateFeature);
-            }
             if (_currentIMemoryPoolFeature != null)
             {
                 yield return new KeyValuePair<Type, object>(typeof(IMemoryPoolFeature), _currentIMemoryPoolFeature);
diff --git a/src/Servers/Kestrel/tools/CodeGenerator/HttpProtocolFeatureCollection.cs b/src/Servers/Kestrel/tools/CodeGenerator/HttpProtocolFeatureCollection.cs
@@ -81,6 +81,7 @@ public static string GenerateFile()
             };
 
             var usings = $@"
+using Microsoft.AspNetCore.Connections.Features;
 using Microsoft.AspNetCore.Http.Features;
 using Microsoft.AspNetCore.Http.Features.Authentication;
 using Microsoft.AspNetCore.Server.Kestrel.Core.Features;";
diff --git a/src/Servers/Kestrel/tools/CodeGenerator/TransportMultiplexedConnectionFeatureCollection.cs b/src/Servers/Kestrel/tools/CodeGenerator/TransportMultiplexedConnectionFeatureCollection.cs
@@ -16,15 +16,13 @@ public static string GenerateFile()
                 "IConnectionIdFeature",
                 "IConnectionTransportFeature",
                 "IConnectionItemsFeature",
-                "IPersistentStateFeature",
                 "IMemoryPoolFeature",
                 "IConnectionLifetimeFeature"
             };
             var implementedFeatures = new[]
             {
                 "IConnectionIdFeature",
                 "IConnectionItemsFeature",
-                "IPersistentStateFeature",
                 "IMemoryPoolFeature",
                 "IConnectionLifetimeFeature"
             };

Original file line number	Diff line number	Diff line change
`@@ -132,6 +132,8 @@ public override string ConnectionId`
`132`	`132`
`133`	`133`	`public void Start()`
`134`	`134`	`{`
	`135`	`+ Debug.Assert(_processingTask.IsCompletedSuccessfully);`
	`136`	`+`
`135`	`137`	`_processingTask = StartAsync();`
`136`	`138`	`}`
`137`	`139`
Original file line number	Diff line number	Diff line change
`@@ -404,7 +404,7 @@ public async Task StreamPool_ManyConcurrentStreams_StreamPoolFull()`
`404`	`404`	`// TODO: Race condition in QUIC library.`
`405`	`405`	`// Delay between sending streams to avoid`
`406`	`406`	`// https://github.com/dotnet/runtime/issues/55249`
`407`		`- await Task.Delay(50);`
	`407`	`+ await Task.Delay(100);`
`408`	`408`	`streamTasks.Add(SendStream(requestState));`
`409`	`409`	`}`
`410`	`410`
Original file line number	Diff line number	Diff line change
`@@ -14,7 +14,7 @@ internal interface IPooledStream`
`14`	`14`	`void DisposeCore();`
`15`	`15`	`}`
`16`	`16`
`17`		`- // See https://github.com/dotnet/runtime/blob/master/src/libraries/System.IO.Pipelines/src/System/IO/Pipelines/BufferSegmentStack.cs`
	`17`	`+ // See https://github.com/dotnet/runtime/blob/da9b16f2804e87c9c1ca9dcd9036e7b53e724f5d/src/libraries/System.IO.Pipelines/src/System/IO/Pipelines/BufferSegmentStack.cs`
`18`	`18`	`internal struct PooledStreamStack<TValue> where TValue : class, IPooledStream`
`19`	`19`	`{`
`20`	`20`	`// Internal for testing`
`@@ -139,6 +139,7 @@ private static int CalculateRemoveCount(long now, int size, StreamAsValueType[]`
`139`	`139`	`return size;`
`140`	`140`	`}`
`141`	`141`
	`142`	`+ // See https://github.com/dotnet/runtime/blob/da9b16f2804e87c9c1ca9dcd9036e7b53e724f5d/src/libraries/System.IO.Pipelines/src/System/IO/Pipelines/BufferSegmentStack.cs#L68-L79`
`142`	`143`	`internal readonly struct StreamAsValueType`
`143`	`144`	`{`
`144`	`145`	`private readonly TValue _value;`
Original file line number	Diff line number	Diff line change
`@@ -16,14 +16,12 @@ namespace Microsoft.AspNetCore.Connections`
`16`	`16`	`internal partial class TransportMultiplexedConnection : IFeatureCollection,`
`17`	`17`	`IConnectionIdFeature,`
`18`	`18`	`IConnectionItemsFeature,`
`19`		`- IPersistentStateFeature,`
`20`	`19`	`IMemoryPoolFeature,`
`21`	`20`	`IConnectionLifetimeFeature`
`22`	`21`	`{`
`23`	`22`	`// Implemented features`
`24`	`23`	`internal protected IConnectionIdFeature? _currentIConnectionIdFeature;`
`25`	`24`	`internal protected IConnectionItemsFeature? _currentIConnectionItemsFeature;`
`26`		`- internal protected IPersistentStateFeature? _currentIPersistentStateFeature;`
`27`	`25`	`internal protected IMemoryPoolFeature? _currentIMemoryPoolFeature;`
`28`	`26`	`internal protected IConnectionLifetimeFeature? _currentIConnectionLifetimeFeature;`
`29`	`27`
`@@ -38,7 +36,6 @@ private void FastReset()`
`38`	`36`	`{`
`39`	`37`	`_currentIConnectionIdFeature = this;`
`40`	`38`	`_currentIConnectionItemsFeature = this;`
`41`		`- _currentIPersistentStateFeature = this;`
`42`	`39`	`_currentIMemoryPoolFeature = this;`
`43`	`40`	`_currentIConnectionLifetimeFeature = this;`
`44`	`41`
`@@ -126,10 +123,6 @@ private void ExtraFeatureSet(Type key, object? value)`
`126`	`123`	`{`
`127`	`124`	`feature = _currentIConnectionItemsFeature;`
`128`	`125`	`}`
`129`		`- else if (key == typeof(IPersistentStateFeature))`
`130`		`- {`
`131`		`- feature = _currentIPersistentStateFeature;`
`132`		`- }`
`133`	`126`	`else if (key == typeof(IMemoryPoolFeature))`
`134`	`127`	`{`
`135`	`128`	`feature = _currentIMemoryPoolFeature;`
`@@ -162,10 +155,6 @@ private void ExtraFeatureSet(Type key, object? value)`
`162`	`155`	`{`
`163`	`156`	`_currentIConnectionItemsFeature = (IConnectionItemsFeature?)value;`
`164`	`157`	`}`
`165`		`- else if (key == typeof(IPersistentStateFeature))`
`166`		`- {`
`167`		`- _currentIPersistentStateFeature = (IPersistentStateFeature?)value;`
`168`		`- }`
`169`	`158`	`else if (key == typeof(IMemoryPoolFeature))`
`170`	`159`	`{`
`171`	`160`	`_currentIMemoryPoolFeature = (IMemoryPoolFeature?)value;`
`@@ -200,10 +189,6 @@ private void ExtraFeatureSet(Type key, object? value)`
`200`	`189`	`{`
`201`	`190`	`feature = Unsafe.As<IConnectionItemsFeature?, TFeature?>(ref _currentIConnectionItemsFeature);`
`202`	`191`	`}`
`203`		`- else if (typeof(TFeature) == typeof(IPersistentStateFeature))`
`204`		`- {`
`205`		`- feature = Unsafe.As<IPersistentStateFeature?, TFeature?>(ref _currentIPersistentStateFeature);`
`206`		`- }`
`207`	`192`	`else if (typeof(TFeature) == typeof(IMemoryPoolFeature))`
`208`	`193`	`{`
`209`	`194`	`feature = Unsafe.As<IMemoryPoolFeature?, TFeature?>(ref _currentIMemoryPoolFeature);`
`@@ -239,10 +224,6 @@ private void ExtraFeatureSet(Type key, object? value)`
`239`	`224`	`{`
`240`	`225`	`_currentIConnectionItemsFeature = Unsafe.As<TFeature?, IConnectionItemsFeature?>(ref feature);`
`241`	`226`	`}`
`242`		`- else if (typeof(TFeature) == typeof(IPersistentStateFeature))`
`243`		`- {`
`244`		`- _currentIPersistentStateFeature = Unsafe.As<TFeature?, IPersistentStateFeature?>(ref feature);`
`245`		`- }`
`246`	`227`	`else if (typeof(TFeature) == typeof(IMemoryPoolFeature))`
`247`	`228`	`{`
`248`	`229`	`_currentIMemoryPoolFeature = Unsafe.As<TFeature?, IMemoryPoolFeature?>(ref feature);`
`@@ -271,10 +252,6 @@ private IEnumerable<KeyValuePair<Type, object>> FastEnumerable()`
`271`	`252`	`{`
`272`	`253`	`yield return new KeyValuePair<Type, object>(typeof(IConnectionItemsFeature), _currentIConnectionItemsFeature);`
`273`	`254`	`}`
`274`		`- if (_currentIPersistentStateFeature != null)`
`275`		`- {`
`276`		`- yield return new KeyValuePair<Type, object>(typeof(IPersistentStateFeature), _currentIPersistentStateFeature);`
`277`		`- }`
`278`	`255`	`if (_currentIMemoryPoolFeature != null)`
`279`	`256`	`{`
`280`	`257`	`yield return new KeyValuePair<Type, object>(typeof(IMemoryPoolFeature), _currentIMemoryPoolFeature);`