From f652c222028ebd504203dce6a68e67b416465aab Mon Sep 17 00:00:00 2001 From: Javier Calvarro Nelson Date: Sat, 15 Jun 2019 00:54:14 +0200 Subject: [PATCH] [Fixes #11183] Race condition in RouteBase.EnsureLoggers (#11218) [Fixes #11183] Race condition in RouteBase.EnsureLoggers --- src/Http/Routing/src/RouteBase.cs | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/src/Http/Routing/src/RouteBase.cs b/src/Http/Routing/src/RouteBase.cs index c12eaae75b..75d393a5d4 100644 --- a/src/Http/Routing/src/RouteBase.cs +++ b/src/Http/Routing/src/RouteBase.cs @@ -3,20 +3,19 @@ using System; using System.Collections.Generic; -using System.Text.Encodings.Web; using System.Threading.Tasks; using Microsoft.AspNetCore.Http; -using Microsoft.AspNetCore.Routing.Internal; using Microsoft.AspNetCore.Routing.Logging; using Microsoft.AspNetCore.Routing.Template; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; -using Microsoft.Extensions.ObjectPool; namespace Microsoft.AspNetCore.Routing { public abstract class RouteBase : IRouter, INamedRouter { + private readonly object _loggersLock = new object(); + private TemplateMatcher _matcher; private TemplateBinder _binder; private ILogger _logger; @@ -259,11 +258,25 @@ namespace Microsoft.AspNetCore.Routing private void EnsureLoggers(HttpContext context) { + // We check first using the _logger to see if the loggers have been initialized to avoid taking + // the lock on the most common case. if (_logger == null) { - var factory = context.RequestServices.GetRequiredService(); - _logger = factory.CreateLogger(typeof(RouteBase).FullName); - _constraintLogger = factory.CreateLogger(typeof(RouteConstraintMatcher).FullName); + // We need to lock here to ensure that _constraintLogger and _logger get initialized atomically. + lock (_loggersLock) + { + if (_logger != null) + { + // Multiple threads might have tried to accquire the lock at the same time. Technically + // there is nothing wrong if things get reinitialized by a second thread, but its easy + // to prevent by just rechecking and returning here. + return; + } + + var factory = context.RequestServices.GetRequiredService(); + _constraintLogger = factory.CreateLogger(typeof(RouteConstraintMatcher).FullName); + _logger = factory.CreateLogger(typeof(RouteBase).FullName); + } } }