Fix Routing#837
Fixes a few issues with how we initialize the middleare. - Always completes the intitialization task - Avoids capturing the ExecutionContext - Allows initialization to occur repeatedly when it fails
This commit is contained in:
parent
af47b27f0d
commit
81fb93a896
|
|
@ -95,23 +95,51 @@ namespace Microsoft.AspNetCore.Routing
|
||||||
// blocking operation if you have a low core count and enough work to do.
|
// blocking operation if you have a low core count and enough work to do.
|
||||||
private Task<Matcher> InitializeAsync()
|
private Task<Matcher> InitializeAsync()
|
||||||
{
|
{
|
||||||
if (_initializationTask != null)
|
var initializationTask = _initializationTask;
|
||||||
|
if (initializationTask != null)
|
||||||
{
|
{
|
||||||
return _initializationTask;
|
return initializationTask;
|
||||||
}
|
}
|
||||||
|
|
||||||
var initializationTask = new TaskCompletionSource<Matcher>();
|
return InitializeCoreAsync();
|
||||||
if (Interlocked.CompareExchange<Task<Matcher>>(
|
}
|
||||||
ref _initializationTask,
|
|
||||||
initializationTask.Task,
|
private Task<Matcher> InitializeCoreAsync()
|
||||||
null) == null)
|
{
|
||||||
|
var initialization = new TaskCompletionSource<Matcher>(TaskCreationOptions.RunContinuationsAsynchronously);
|
||||||
|
var initializationTask = Interlocked.CompareExchange(ref _initializationTask, initialization.Task, null);
|
||||||
|
if (initializationTask != null)
|
||||||
|
{
|
||||||
|
// This thread lost the race, join the existing task.
|
||||||
|
return initializationTask;
|
||||||
|
}
|
||||||
|
|
||||||
|
// This thread won the race, do the initialization.
|
||||||
|
try
|
||||||
{
|
{
|
||||||
// This thread won the race, do the initialization.
|
|
||||||
var matcher = _matcherFactory.CreateMatcher(_endpointDataSource);
|
var matcher = _matcherFactory.CreateMatcher(_endpointDataSource);
|
||||||
initializationTask.SetResult(matcher);
|
|
||||||
}
|
|
||||||
|
|
||||||
return _initializationTask;
|
// Now replace the initialization task with one created with the default execution context.
|
||||||
|
// This is important because capturing the execution context will leak memory in ASP.NET Core.
|
||||||
|
using (ExecutionContext.SuppressFlow())
|
||||||
|
{
|
||||||
|
_initializationTask = Task.FromResult(matcher);
|
||||||
|
}
|
||||||
|
|
||||||
|
// Complete the task, this will unblock any requests that came in while initializing.
|
||||||
|
initialization.SetResult(matcher);
|
||||||
|
return initialization.Task;
|
||||||
|
}
|
||||||
|
catch (Exception ex)
|
||||||
|
{
|
||||||
|
// Allow initialization to occur again. Since DataSources can change, it's possible
|
||||||
|
// for the developer to correct the data causing the failure.
|
||||||
|
_initializationTask = null;
|
||||||
|
|
||||||
|
// Complete the task, this will throw for any requests that came in while initializing.
|
||||||
|
initialization.SetException(ex);
|
||||||
|
return initialization.Task;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
private static class Log
|
private static class Log
|
||||||
|
|
|
||||||
|
|
@ -2,14 +2,17 @@
|
||||||
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
|
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
|
||||||
|
|
||||||
using System;
|
using System;
|
||||||
|
using System.Threading;
|
||||||
using System.Threading.Tasks;
|
using System.Threading.Tasks;
|
||||||
using Microsoft.AspNetCore.Http;
|
using Microsoft.AspNetCore.Http;
|
||||||
using Microsoft.AspNetCore.Http.Features;
|
using Microsoft.AspNetCore.Http.Features;
|
||||||
|
using Microsoft.AspNetCore.Routing.Matching;
|
||||||
using Microsoft.AspNetCore.Routing.TestObjects;
|
using Microsoft.AspNetCore.Routing.TestObjects;
|
||||||
using Microsoft.Extensions.Logging;
|
using Microsoft.Extensions.Logging;
|
||||||
using Microsoft.Extensions.Logging.Abstractions;
|
using Microsoft.Extensions.Logging.Abstractions;
|
||||||
using Microsoft.Extensions.Logging.Testing;
|
using Microsoft.Extensions.Logging.Testing;
|
||||||
using Microsoft.Extensions.Options;
|
using Microsoft.Extensions.Options;
|
||||||
|
using Moq;
|
||||||
using Xunit;
|
using Xunit;
|
||||||
|
|
||||||
namespace Microsoft.AspNetCore.Routing
|
namespace Microsoft.AspNetCore.Routing
|
||||||
|
|
@ -103,6 +106,29 @@ namespace Microsoft.AspNetCore.Routing
|
||||||
Assert.Equal("testValue", routeValuesFeature.RouteValues["testKey"]);
|
Assert.Equal("testValue", routeValuesFeature.RouteValues["testKey"]);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task Invoke_InitializationFailure_AllowsReinitialization()
|
||||||
|
{
|
||||||
|
// Arrange
|
||||||
|
var httpContext = CreateHttpContext();
|
||||||
|
|
||||||
|
var matcherFactory = new Mock<MatcherFactory>();
|
||||||
|
matcherFactory
|
||||||
|
.Setup(f => f.CreateMatcher(It.IsAny<EndpointDataSource>()))
|
||||||
|
.Throws(new InvalidTimeZoneException())
|
||||||
|
.Verifiable();
|
||||||
|
|
||||||
|
var middleware = CreateMiddleware(matcherFactory: matcherFactory.Object);
|
||||||
|
|
||||||
|
// Act
|
||||||
|
await Assert.ThrowsAsync<InvalidTimeZoneException>(async () => await middleware.Invoke(httpContext));
|
||||||
|
await Assert.ThrowsAsync<InvalidTimeZoneException>(async () => await middleware.Invoke(httpContext));
|
||||||
|
|
||||||
|
// Assert
|
||||||
|
matcherFactory
|
||||||
|
.Verify(f => f.CreateMatcher(It.IsAny<EndpointDataSource>()), Times.Exactly(2));
|
||||||
|
}
|
||||||
|
|
||||||
private HttpContext CreateHttpContext()
|
private HttpContext CreateHttpContext()
|
||||||
{
|
{
|
||||||
var context = new EndpointSelectorContext();
|
var context = new EndpointSelectorContext();
|
||||||
|
|
@ -116,14 +142,16 @@ namespace Microsoft.AspNetCore.Routing
|
||||||
return httpContext;
|
return httpContext;
|
||||||
}
|
}
|
||||||
|
|
||||||
private EndpointRoutingMiddleware CreateMiddleware(Logger<EndpointRoutingMiddleware> logger = null)
|
private EndpointRoutingMiddleware CreateMiddleware(
|
||||||
|
Logger<EndpointRoutingMiddleware> logger = null,
|
||||||
|
MatcherFactory matcherFactory = null)
|
||||||
{
|
{
|
||||||
RequestDelegate next = (c) => Task.FromResult<object>(null);
|
RequestDelegate next = (c) => Task.FromResult<object>(null);
|
||||||
|
|
||||||
logger = logger ?? new Logger<EndpointRoutingMiddleware>(NullLoggerFactory.Instance);
|
logger = logger ?? new Logger<EndpointRoutingMiddleware>(NullLoggerFactory.Instance);
|
||||||
|
matcherFactory = matcherFactory ?? new TestMatcherFactory(true);
|
||||||
|
|
||||||
var options = Options.Create(new EndpointOptions());
|
var options = Options.Create(new EndpointOptions());
|
||||||
var matcherFactory = new TestMatcherFactory(true);
|
|
||||||
var middleware = new EndpointRoutingMiddleware(
|
var middleware = new EndpointRoutingMiddleware(
|
||||||
matcherFactory,
|
matcherFactory,
|
||||||
new CompositeEndpointDataSource(Array.Empty<EndpointDataSource>()),
|
new CompositeEndpointDataSource(Array.Empty<EndpointDataSource>()),
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue