Skip to content

Commit 03b75e3

Browse files
authored
Inline second statemahine into loop and us EC.Restore to reset context (#23175)
1 parent de726cc commit 03b75e3

File tree

1 file changed

+81
-84
lines changed

1 file changed

+81
-84
lines changed

src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs

Lines changed: 81 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -607,6 +607,7 @@ public async Task ProcessRequestsAsync<TContext>(IHttpApplication<TContext> appl
607607

608608
private async Task ProcessRequests<TContext>(IHttpApplication<TContext> application)
609609
{
610+
var cleanContext = ExecutionContext.Capture();
610611
while (_keepAlive)
611612
{
612613
BeginRequestProcessing();
@@ -637,111 +638,107 @@ private async Task ProcessRequests<TContext>(IHttpApplication<TContext> applicat
637638

638639
InitializeBodyControl(messageBody);
639640

640-
// We run user controlled request processing in a seperate async method
641-
// so any changes made to ExecutionContext are undone when it returns and
642-
// each request starts with a fresh ExecutionContext state.
643-
await ProcessRequest(application);
644641

645-
// Even for non-keep-alive requests, try to consume the entire body to avoid RSTs.
646-
if (!_connectionAborted && _requestRejectedException == null && !messageBody.IsEmpty)
647-
{
648-
await messageBody.ConsumeAsync();
649-
}
642+
var context = application.CreateContext(this);
650643

651-
if (HasStartedConsumingRequestBody)
644+
try
652645
{
653-
await messageBody.StopAsync();
654-
}
655-
}
656-
}
646+
KestrelEventSource.Log.RequestStart(this);
657647

658-
private async ValueTask ProcessRequest<TContext>(IHttpApplication<TContext> application)
659-
{
660-
var context = application.CreateContext(this);
648+
// Run the application code for this request
649+
await application.ProcessRequestAsync(context);
661650

662-
try
663-
{
664-
KestrelEventSource.Log.RequestStart(this);
651+
// Trigger OnStarting if it hasn't been called yet and the app hasn't
652+
// already failed. If an OnStarting callback throws we can go through
653+
// our normal error handling in ProduceEnd.
654+
// https://github.com/aspnet/KestrelHttpServer/issues/43
655+
if (!HasResponseStarted && _applicationException == null && _onStarting?.Count > 0)
656+
{
657+
await FireOnStarting();
658+
}
665659

666-
// Run the application code for this request
667-
await application.ProcessRequestAsync(context);
660+
if (!_connectionAborted && !VerifyResponseContentLength(out var lengthException))
661+
{
662+
ReportApplicationError(lengthException);
663+
}
664+
}
665+
catch (BadHttpRequestException ex)
666+
{
667+
// Capture BadHttpRequestException for further processing
668+
// This has to be caught here so StatusCode is set properly before disposing the HttpContext
669+
// (DisposeContext logs StatusCode).
670+
SetBadRequestState(ex);
671+
ReportApplicationError(ex);
672+
}
673+
catch (Exception ex)
674+
{
675+
ReportApplicationError(ex);
676+
}
668677

669-
// Trigger OnStarting if it hasn't been called yet and the app hasn't
670-
// already failed. If an OnStarting callback throws we can go through
671-
// our normal error handling in ProduceEnd.
672-
// https://github.com/aspnet/KestrelHttpServer/issues/43
673-
if (!HasResponseStarted && _applicationException == null && _onStarting?.Count > 0)
678+
KestrelEventSource.Log.RequestStop(this);
679+
680+
// At this point all user code that needs use to the request or response streams has completed.
681+
// Using these streams in the OnCompleted callback is not allowed.
682+
try
674683
{
675-
await FireOnStarting();
684+
await _bodyControl.StopAsync();
685+
}
686+
catch (Exception ex)
687+
{
688+
// BodyControl.StopAsync() can throw if the PipeWriter was completed prior to the application writing
689+
// enough bytes to satisfy the specified Content-Length. This risks double-logging the exception,
690+
// but this scenario generally indicates an app bug, so I don't want to risk not logging it.
691+
ReportApplicationError(ex);
676692
}
677693

678-
if (!_connectionAborted && !VerifyResponseContentLength(out var lengthException))
694+
// 4XX responses are written by TryProduceInvalidRequestResponse during connection tear down.
695+
if (_requestRejectedException == null)
679696
{
680-
ReportApplicationError(lengthException);
697+
if (!_connectionAborted)
698+
{
699+
// Call ProduceEnd() before consuming the rest of the request body to prevent
700+
// delaying clients waiting for the chunk terminator:
701+
//
702+
// https://github.com/dotnet/corefx/issues/17330#issuecomment-288248663
703+
//
704+
// This also prevents the 100 Continue response from being sent if the app
705+
// never tried to read the body.
706+
// https://github.com/aspnet/KestrelHttpServer/issues/2102
707+
//
708+
// ProduceEnd() must be called before _application.DisposeContext(), to ensure
709+
// HttpContext.Response.StatusCode is correctly set when
710+
// IHttpContextFactory.Dispose(HttpContext) is called.
711+
await ProduceEnd();
712+
}
713+
else if (!HasResponseStarted)
714+
{
715+
// If the request was aborted and no response was sent, there's no
716+
// meaningful status code to log.
717+
StatusCode = 0;
718+
}
681719
}
682-
}
683-
catch (BadHttpRequestException ex)
684-
{
685-
// Capture BadHttpRequestException for further processing
686-
// This has to be caught here so StatusCode is set properly before disposing the HttpContext
687-
// (DisposeContext logs StatusCode).
688-
SetBadRequestState(ex);
689-
ReportApplicationError(ex);
690-
}
691-
catch (Exception ex)
692-
{
693-
ReportApplicationError(ex);
694-
}
695720

696-
KestrelEventSource.Log.RequestStop(this);
721+
if (_onCompleted?.Count > 0)
722+
{
723+
await FireOnCompleted();
724+
}
697725

698-
// At this point all user code that needs use to the request or response streams has completed.
699-
// Using these streams in the OnCompleted callback is not allowed.
700-
try
701-
{
702-
await _bodyControl.StopAsync();
703-
}
704-
catch (Exception ex)
705-
{
706-
// BodyControl.StopAsync() can throw if the PipeWriter was completed prior to the application writing
707-
// enough bytes to satisfy the specified Content-Length. This risks double-logging the exception,
708-
// but this scenario generally indicates an app bug, so I don't want to risk not logging it.
709-
ReportApplicationError(ex);
710-
}
726+
application.DisposeContext(context, _applicationException);
711727

712-
// 4XX responses are written by TryProduceInvalidRequestResponse during connection tear down.
713-
if (_requestRejectedException == null)
714-
{
715-
if (!_connectionAborted)
728+
// Even for non-keep-alive requests, try to consume the entire body to avoid RSTs.
729+
if (!_connectionAborted && _requestRejectedException == null && !messageBody.IsEmpty)
716730
{
717-
// Call ProduceEnd() before consuming the rest of the request body to prevent
718-
// delaying clients waiting for the chunk terminator:
719-
//
720-
// https://github.com/dotnet/corefx/issues/17330#issuecomment-288248663
721-
//
722-
// This also prevents the 100 Continue response from being sent if the app
723-
// never tried to read the body.
724-
// https://github.com/aspnet/KestrelHttpServer/issues/2102
725-
//
726-
// ProduceEnd() must be called before _application.DisposeContext(), to ensure
727-
// HttpContext.Response.StatusCode is correctly set when
728-
// IHttpContextFactory.Dispose(HttpContext) is called.
729-
await ProduceEnd();
731+
await messageBody.ConsumeAsync();
730732
}
731-
else if (!HasResponseStarted)
733+
734+
if (HasStartedConsumingRequestBody)
732735
{
733-
// If the request was aborted and no response was sent, there's no
734-
// meaningful status code to log.
735-
StatusCode = 0;
736+
await messageBody.StopAsync();
736737
}
737-
}
738738

739-
if (_onCompleted?.Count > 0)
740-
{
741-
await FireOnCompleted();
739+
// Clear any AsyncLocals set during the request; back to a clean state ready for next request
740+
ExecutionContext.Restore(cleanContext);
742741
}
743-
744-
application.DisposeContext(context, _applicationException);
745742
}
746743

747744
public void OnStarting(Func<object, Task> callback, object state)

0 commit comments

Comments
 (0)