Use a single request object (#28347)

- Merged NativeRequestContext, RequestContext and FeatureContext into a single object.

Author: davidfowl

src/Servers/HttpSys/HttpSysServer.slnf

@@ -23,7 +23,8 @@
       "src\\Servers\\HttpSys\\src\\Microsoft.AspNetCore.Server.HttpSys.csproj",
       "src\\Servers\\HttpSys\\test\\FunctionalTests\\Microsoft.AspNetCore.Server.HttpSys.FunctionalTests.csproj",
       "src\\Servers\\HttpSys\\test\\Tests\\Microsoft.AspNetCore.Server.HttpSys.Tests.csproj",
-      "src\\Servers\\Kestrel\\Transport.Sockets\\src\\Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets.csproj"
+      "src\\Servers\\Kestrel\\Transport.Sockets\\src\\Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets.csproj",
+      "src\\Testing\\src\\Microsoft.AspNetCore.Testing.csproj"
     ]
   }
 }

src/Servers/HttpSys/src/AsyncAcceptContext.cs

@@ -23,7 +23,7 @@ internal unsafe class AsyncAcceptContext : IValueTaskSource<RequestContext>, IDi
             RunContinuationsAsynchronously = false
         };
 
-        private NativeRequestContext _nativeRequestContext;
+        private RequestContext _requestContext;
 
         internal AsyncAcceptContext(HttpSysListener server)
         {
@@ -53,14 +53,18 @@ internal ValueTask<RequestContext> AcceptAsync()
 
         private static void IOCompleted(AsyncAcceptContext asyncContext, uint errorCode, uint numBytes)
         {
-            bool complete = false;
+            var complete = false;
+            // This is important to stash a ref to as it's a mutable struct
+            ref var mrvts = ref asyncContext._mrvts;
+            var requestContext = asyncContext._requestContext;
+            var requestId = requestContext.RequestId;
 
             try
             {
                 if (errorCode != UnsafeNclNativeMethods.ErrorCodes.ERROR_SUCCESS &&
                     errorCode != UnsafeNclNativeMethods.ErrorCodes.ERROR_MORE_DATA)
                 {
-                    asyncContext._mrvts.SetException(new HttpSysException((int)errorCode));
+                    mrvts.SetException(new HttpSysException((int)errorCode));
                     return;
                 }
 
@@ -71,37 +75,39 @@ private static void IOCompleted(AsyncAcceptContext asyncContext, uint errorCode,
                     // points to it we need to hook up our authentication handling code here.
                     try
                     {
-                        var nativeContext = asyncContext._nativeRequestContext;
-
-                        if (server.ValidateRequest(nativeContext) && server.ValidateAuth(nativeContext))
+                        if (server.ValidateRequest(requestContext) && server.ValidateAuth(requestContext))
                         {
-                            // It's important that we clear the native request context before we set the result
-                            // we want to reuse this object for future accepts.
-                            asyncContext._nativeRequestContext = null;
+                            // It's important that we clear the request context before we set the result
+                            // we want to reuse the acceptContext object for future accepts.
+                            asyncContext._requestContext = null;
+
+                            // Initialize features here once we're successfully validated the request
+                            // TODO: In the future defer this work to the thread pool so we can get off the IO thread
+                            // as quickly as possible
+                            requestContext.InitializeFeatures();
 
-                            var requestContext = new RequestContext(server, nativeContext);
-                            asyncContext._mrvts.SetResult(requestContext);
+                            mrvts.SetResult(requestContext);
 
                             complete = true;
                         }
                     }
                     catch (Exception ex)
                     {
-                        server.SendError(asyncContext._nativeRequestContext.RequestId, StatusCodes.Status400BadRequest);
-                        asyncContext._mrvts.SetException(ex);
+                        server.SendError(requestId, StatusCodes.Status400BadRequest);
+                        mrvts.SetException(ex);
                     }
                     finally
                     {
                         if (!complete)
                         {
-                            asyncContext.AllocateNativeRequest(size: asyncContext._nativeRequestContext.Size);
+                            asyncContext.AllocateNativeRequest(size: requestContext.Size);
                         }
                     }
                 }
                 else
                 {
                     //  (uint)backingBuffer.Length - AlignmentPadding
-                    asyncContext.AllocateNativeRequest(numBytes, asyncContext._nativeRequestContext.RequestId);
+                    asyncContext.AllocateNativeRequest(numBytes, requestId);
                 }
 
                 // We need to issue a new request, either because auth failed, or because our buffer was too small the first time.
@@ -114,20 +120,20 @@ private static void IOCompleted(AsyncAcceptContext asyncContext, uint errorCode,
                     {
                         // someother bad error, possible(?) return values are:
                         // ERROR_INVALID_HANDLE, ERROR_INSUFFICIENT_BUFFER, ERROR_OPERATION_ABORTED
-                        asyncContext._mrvts.SetException(new HttpSysException((int)statusCode));
+                        mrvts.SetException(new HttpSysException((int)statusCode));
                     }
                 }
             }
             catch (Exception exception)
             {
-                asyncContext._mrvts.SetException(exception);
+                mrvts.SetException(exception);
             }
         }
 
         private static unsafe void IOWaitCallback(uint errorCode, uint numBytes, NativeOverlapped* nativeOverlapped)
         {
-            var asyncResult = (AsyncAcceptContext)ThreadPoolBoundHandle.GetNativeOverlappedState(nativeOverlapped);
-            IOCompleted(asyncResult, errorCode, numBytes);
+            var acceptContext = (AsyncAcceptContext)ThreadPoolBoundHandle.GetNativeOverlappedState(nativeOverlapped);
+            IOCompleted(acceptContext, errorCode, numBytes);
         }
 
         private uint QueueBeginGetContext()
@@ -140,21 +146,21 @@ private uint QueueBeginGetContext()
                 uint bytesTransferred = 0;
                 statusCode = HttpApi.HttpReceiveHttpRequest(
                     Server.RequestQueue.Handle,
-                    _nativeRequestContext.RequestId,
+                    _requestContext.RequestId,
                     // Small perf impact by not using HTTP_RECEIVE_REQUEST_FLAG_COPY_BODY
                     // if the request sends header+body in a single TCP packet 
                     (uint)HttpApiTypes.HTTP_FLAGS.NONE,
-                    _nativeRequestContext.NativeRequest,
-                    _nativeRequestContext.Size,
+                    _requestContext.NativeRequest,
+                    _requestContext.Size,
                     &bytesTransferred,
                     _overlapped);
 
-                if (statusCode == UnsafeNclNativeMethods.ErrorCodes.ERROR_INVALID_PARAMETER && _nativeRequestContext.RequestId != 0)
+                if (statusCode == UnsafeNclNativeMethods.ErrorCodes.ERROR_INVALID_PARAMETER && _requestContext.RequestId != 0)
                 {
                     // we might get this if somebody stole our RequestId,
                     // set RequestId to 0 and start all over again with the buffer we just allocated
                     // BUGBUG: how can someone steal our request ID?  seems really bad and in need of fix.
-                    _nativeRequestContext.RequestId = 0;
+                    _requestContext.RequestId = 0;
                     retry = true;
                 }
                 else if (statusCode == UnsafeNclNativeMethods.ErrorCodes.ERROR_MORE_DATA)
@@ -178,17 +184,16 @@ private uint QueueBeginGetContext()
 
         private void AllocateNativeRequest(uint? size = null, ulong requestId = 0)
         {
-            _nativeRequestContext?.ReleasePins();
-            _nativeRequestContext?.Dispose();
+            _requestContext?.ReleasePins();
+            _requestContext?.Dispose();
 
             var boundHandle = Server.RequestQueue.BoundHandle;
-
             if (_overlapped != null)
             {
                 boundHandle.FreeNativeOverlapped(_overlapped);
             }
 
-            _nativeRequestContext = new NativeRequestContext(Server.MemoryPool, size, requestId);
+            _requestContext = new RequestContext(Server, size, requestId);
             _overlapped = boundHandle.AllocateNativeOverlapped(_preallocatedOverlapped);
         }
 
@@ -201,11 +206,11 @@ protected virtual void Dispose(bool disposing)
         {
             if (disposing)
             {
-                if (_nativeRequestContext != null)
+                if (_requestContext != null)
                 {
-                    _nativeRequestContext.ReleasePins();
-                    _nativeRequestContext.Dispose();
-                    _nativeRequestContext = null;
+                    _requestContext.ReleasePins();
+                    _requestContext.Dispose();
+                    _requestContext = null;
 
                     var boundHandle = Server.RequestQueue.BoundHandle;
 

src/Servers/HttpSys/src/RequestProcessing/Request.cs

@@ -20,8 +20,6 @@ namespace Microsoft.AspNetCore.Server.HttpSys
 {
     internal sealed class Request
     {
-        private NativeRequestContext _nativeRequestContext;
-
         private X509Certificate2 _clientCert;
         // TODO: https://github.com/aspnet/HttpSysServer/issues/231
         // private byte[] _providedTokenBindingId;
@@ -39,26 +37,25 @@ internal sealed class Request
 
         private bool _isDisposed = false;
 
-        internal Request(RequestContext requestContext, NativeRequestContext nativeRequestContext)
+        internal Request(RequestContext requestContext)
         {
             // TODO: Verbose log
             RequestContext = requestContext;
-            _nativeRequestContext = nativeRequestContext;
             _contentBoundaryType = BoundaryType.None;
 
-            RequestId = nativeRequestContext.RequestId;
-            UConnectionId = nativeRequestContext.ConnectionId;
-            SslStatus = nativeRequestContext.SslStatus;
+            RequestId = requestContext.RequestId;
+            UConnectionId = requestContext.ConnectionId;
+            SslStatus = requestContext.SslStatus;
 
-            KnownMethod = nativeRequestContext.VerbId;
-            Method = _nativeRequestContext.GetVerb();
+            KnownMethod = requestContext.VerbId;
+            Method = requestContext.GetVerb();
 
-            RawUrl = nativeRequestContext.GetRawUrl();
+            RawUrl = requestContext.GetRawUrl();
 
-            var cookedUrl = nativeRequestContext.GetCookedUrl();
+            var cookedUrl = requestContext.GetCookedUrl();
             QueryString = cookedUrl.GetQueryString() ?? string.Empty;
 
-            var rawUrlInBytes = _nativeRequestContext.GetRawUrlInBytes();
+            var rawUrlInBytes = requestContext.GetRawUrlInBytes();
             var originalPath = RequestUriBuilder.DecodeAndUnescapePath(rawUrlInBytes);
 
             PathBase = string.Empty;
@@ -72,7 +69,7 @@ internal Request(RequestContext requestContext, NativeRequestContext nativeReque
             }
             else
             {
-                var prefix = requestContext.Server.Options.UrlPrefixes.GetPrefix((int)nativeRequestContext.UrlContext);
+                var prefix = requestContext.Server.Options.UrlPrefixes.GetPrefix((int)requestContext.UrlContext);
                 // Prefix may be null if the requested has been transfered to our queue
                 if (!(prefix is null))
                 {
@@ -97,11 +94,11 @@ internal Request(RequestContext requestContext, NativeRequestContext nativeReque
                 }
             }
 
-            ProtocolVersion = _nativeRequestContext.GetVersion();
+            ProtocolVersion = RequestContext.GetVersion();
 
-            Headers = new RequestHeaders(_nativeRequestContext);
+            Headers = new RequestHeaders(RequestContext);
 
-            User = _nativeRequestContext.GetUser();
+            User = RequestContext.GetUser();
 
             if (IsHttps)
             {
@@ -111,7 +108,7 @@ internal Request(RequestContext requestContext, NativeRequestContext nativeReque
             // GetTlsTokenBindingInfo(); TODO: https://github.com/aspnet/HttpSysServer/issues/231
 
             // Finished directly accessing the HTTP_REQUEST structure.
-            _nativeRequestContext.ReleasePins();
+            RequestContext.ReleasePins();
             // TODO: Verbose log parameters
         }
 
@@ -222,7 +219,7 @@ private AspNetCore.HttpSys.Internal.SocketAddress RemoteEndPoint
             {
                 if (_remoteEndPoint == null)
                 {
-                    _remoteEndPoint = _nativeRequestContext.GetRemoteEndPoint();
+                    _remoteEndPoint = RequestContext.GetRemoteEndPoint();
                 }
 
                 return _remoteEndPoint;
@@ -235,7 +232,7 @@ private AspNetCore.HttpSys.Internal.SocketAddress LocalEndPoint
             {
                 if (_localEndPoint == null)
                 {
-                    _localEndPoint = _nativeRequestContext.GetLocalEndPoint();
+                    _localEndPoint = RequestContext.GetLocalEndPoint();
                 }
 
                 return _localEndPoint;
@@ -278,15 +275,15 @@ public IReadOnlyDictionary<int, ReadOnlyMemory<byte>> RequestInfo
             {
                 if (_requestInfo == null)
                 {
-                    _requestInfo = _nativeRequestContext.GetRequestInfo();
+                    _requestInfo = RequestContext.GetRequestInfo();
                 }
                 return _requestInfo;
             }
         }
 
         private void GetTlsHandshakeResults()
         {
-            var handshake = _nativeRequestContext.GetTlsHandshake();
+            var handshake = RequestContext.GetTlsHandshake();
 
             Protocol = handshake.Protocol;
             // The OS considers client and server TLS as different enum values. SslProtocols choose to combine those for some reason.
@@ -332,7 +329,7 @@ public X509Certificate2 ClientCertificate
                 {
                     try
                     {
-                        _clientCert = _nativeRequestContext.GetClientCertificate();
+                        _clientCert = RequestContext.GetClientCertificate();
                     }
                     catch (CryptographicException ce)
                     {
@@ -426,27 +423,19 @@ private unsafe void GetTlsTokenBindingInfo()
         */
         internal uint GetChunks(ref int dataChunkIndex, ref uint dataChunkOffset, byte[] buffer, int offset, int size)
         {
-            return _nativeRequestContext.GetChunks(ref dataChunkIndex, ref dataChunkOffset, buffer, offset, size);
+            return RequestContext.GetChunks(ref dataChunkIndex, ref dataChunkOffset, buffer, offset, size);
         }
 
         // should only be called from RequestContext
         internal void Dispose()
         {
-            // TODO: Verbose log
-            _isDisposed = true;
-            _nativeRequestContext.Dispose();
-            (User?.Identity as WindowsIdentity)?.Dispose();
-            if (_nativeStream != null)
-            {
-                _nativeStream.Dispose();
-            }
-        }
-
-        private void CheckDisposed()
-        {
-            if (_isDisposed)
+            if (!_isDisposed)
             {
-                throw new ObjectDisposedException(this.GetType().FullName);
+                // TODO: Verbose log
+                _isDisposed = true;
+                RequestContext.Dispose();
+                (User?.Identity as WindowsIdentity)?.Dispose();
+                _nativeStream?.Dispose();
             }
         }