Skip to content

Use a single request object #28347

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
1 commit merged into from
Dec 5, 2020
Merged

Use a single request object #28347

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/Servers/HttpSys/HttpSysServer.slnf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
]
}
}
69 changes: 37 additions & 32 deletions src/Servers/HttpSys/src/AsyncAcceptContext.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ internal unsafe class AsyncAcceptContext : IValueTaskSource<RequestContext>, IDi
RunContinuationsAsynchronously = false
};

private NativeRequestContext _nativeRequestContext;
private RequestContext _requestContext;

internal AsyncAcceptContext(HttpSysListener server)
{
Expand Down Expand Up @@ -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;
}

Expand All @@ -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.
Expand All @@ -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()
Expand All @@ -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)
Expand All @@ -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);
}

Expand All @@ -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;

Expand Down
63 changes: 26 additions & 37 deletions src/Servers/HttpSys/src/RequestProcessing/Request.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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))
{
Expand All @@ -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)
{
Expand All @@ -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
}

Expand Down Expand Up @@ -222,7 +219,7 @@ private AspNetCore.HttpSys.Internal.SocketAddress RemoteEndPoint
{
if (_remoteEndPoint == null)
{
_remoteEndPoint = _nativeRequestContext.GetRemoteEndPoint();
_remoteEndPoint = RequestContext.GetRemoteEndPoint();
}

return _remoteEndPoint;
Expand All @@ -235,7 +232,7 @@ private AspNetCore.HttpSys.Internal.SocketAddress LocalEndPoint
{
if (_localEndPoint == null)
{
_localEndPoint = _nativeRequestContext.GetLocalEndPoint();
_localEndPoint = RequestContext.GetLocalEndPoint();
}

return _localEndPoint;
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -332,7 +329,7 @@ public X509Certificate2 ClientCertificate
{
try
{
_clientCert = _nativeRequestContext.GetClientCertificate();
_clientCert = RequestContext.GetClientCertificate();
}
catch (CryptographicException ce)
{
Expand Down Expand Up @@ -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();
}
}

Expand Down
Loading