Make constraint cache thread safe (#17146)
* Make constraint cache thread safe Fixes: #17101 This changes the constraint cache to use ConcurrentDictionary. This code is invoked in a multithreaded way in Blazor server resulting in internal failures in dictionary. Since this is a threading issue there's no good way to unit test it, but I noticed we're missing tests in general for this class, so I added a few for the caching behavior. * PR feedback
This commit is contained in:
parent
7ceadd0bcc
commit
8f8b5f55a9
|
|
@ -2,15 +2,20 @@
|
||||||
// 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.Collections.Generic;
|
using System.Collections.Concurrent;
|
||||||
using System.Globalization;
|
using System.Globalization;
|
||||||
|
|
||||||
namespace Microsoft.AspNetCore.Components.Routing
|
namespace Microsoft.AspNetCore.Components.Routing
|
||||||
{
|
{
|
||||||
internal abstract class RouteConstraint
|
internal abstract class RouteConstraint
|
||||||
{
|
{
|
||||||
private static readonly IDictionary<string, RouteConstraint> _cachedConstraints
|
// note: the things that prevent this cache from growing unbounded is that
|
||||||
= new Dictionary<string, RouteConstraint>();
|
// we're the only caller to this code path, and the fact that there are only
|
||||||
|
// 8 possible instances that we create.
|
||||||
|
//
|
||||||
|
// The values passed in here for parsing are always static text defined in route attributes.
|
||||||
|
private static readonly ConcurrentDictionary<string, RouteConstraint> _cachedConstraints
|
||||||
|
= new ConcurrentDictionary<string, RouteConstraint>();
|
||||||
|
|
||||||
public abstract bool Match(string pathSegment, out object convertedValue);
|
public abstract bool Match(string pathSegment, out object convertedValue);
|
||||||
|
|
||||||
|
|
@ -30,8 +35,10 @@ namespace Microsoft.AspNetCore.Components.Routing
|
||||||
var newInstance = CreateRouteConstraint(constraint);
|
var newInstance = CreateRouteConstraint(constraint);
|
||||||
if (newInstance != null)
|
if (newInstance != null)
|
||||||
{
|
{
|
||||||
_cachedConstraints[constraint] = newInstance;
|
// We've done to the work to create the constraint now, but it's possible
|
||||||
return newInstance;
|
// we're competing with another thread. GetOrAdd can ensure only a single
|
||||||
|
// instance is returned so that any extra ones can be GC'ed.
|
||||||
|
return _cachedConstraints.GetOrAdd(constraint, newInstance);
|
||||||
}
|
}
|
||||||
else
|
else
|
||||||
{
|
{
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1,36 @@
|
||||||
|
// Copyright (c) .NET Foundation. All rights reserved.
|
||||||
|
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
|
||||||
|
|
||||||
|
using Xunit;
|
||||||
|
|
||||||
|
namespace Microsoft.AspNetCore.Components.Routing
|
||||||
|
{
|
||||||
|
public class RouteConstraintTest
|
||||||
|
{
|
||||||
|
[Fact]
|
||||||
|
public void Parse_CreatesDifferentConstraints_ForDifferentKinds()
|
||||||
|
{
|
||||||
|
// Arrange
|
||||||
|
var original = RouteConstraint.Parse("ignore", "ignore", "int");
|
||||||
|
|
||||||
|
// Act
|
||||||
|
var another = RouteConstraint.Parse("ignore", "ignore", "guid");
|
||||||
|
|
||||||
|
// Assert
|
||||||
|
Assert.NotSame(original, another);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void Parse_CachesCreatedConstraint_ForSameKind()
|
||||||
|
{
|
||||||
|
// Arrange
|
||||||
|
var original = RouteConstraint.Parse("ignore", "ignore", "int");
|
||||||
|
|
||||||
|
// Act
|
||||||
|
var another = RouteConstraint.Parse("ignore", "ignore", "int");
|
||||||
|
|
||||||
|
// Assert
|
||||||
|
Assert.Same(original, another);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
Loading…
Reference in New Issue