From e65b7825b23f90549a35efab3482faaf39ceb320 Mon Sep 17 00:00:00 2001 From: Sagilio Date: Sun, 12 Jun 2022 23:55:55 +0800 Subject: [PATCH] fix: Make expression cache thread safe (#261) Signed-off-by: sagilio --- .../Evaluation/IExpressionHandler.cs | 7 +--- Casbin/Caching/ExpressionCache.cs | 22 +---------- Casbin/Caching/ExpressionCachePool.cs | 37 +++---------------- Casbin/Casbin.csproj | 16 ++++---- Casbin/Evaluation/ExpressionHandler.cs | 33 ++++++++++++----- Casbin/Util/Utility.cs | 3 ++ 6 files changed, 45 insertions(+), 73 deletions(-) diff --git a/Casbin/Abstractions/Evaluation/IExpressionHandler.cs b/Casbin/Abstractions/Evaluation/IExpressionHandler.cs index a3188f89..299e69de 100644 --- a/Casbin/Abstractions/Evaluation/IExpressionHandler.cs +++ b/Casbin/Abstractions/Evaluation/IExpressionHandler.cs @@ -1,17 +1,14 @@ using System; -using Casbin.Caching; -using Casbin.Effect; using Casbin.Model; namespace Casbin.Evaluation; public interface IExpressionHandler { - public IExpressionCachePool Cache { get; set; } - public void SetFunction(string name, Delegate function); - public bool Invoke(in EnforceContext context, string expressionString, in TRequest request, in TPolicy policy) + public bool Invoke(in EnforceContext context, string expressionString, in TRequest request, + in TPolicy policy) where TRequest : IRequestValues where TPolicy : IPolicyValues; } diff --git a/Casbin/Caching/ExpressionCache.cs b/Casbin/Caching/ExpressionCache.cs index 3ee5a3b7..fb70c7d0 100644 --- a/Casbin/Caching/ExpressionCache.cs +++ b/Casbin/Caching/ExpressionCache.cs @@ -1,29 +1,11 @@ using System; -using System.Collections.Generic; -using DynamicExpresso; +using System.Collections.Concurrent; namespace Casbin.Caching; -internal class ExpressionCache : IExpressionCache -{ - private readonly Dictionary _cache = new(); - - public bool TryGet(string expressionString, out Lambda lambda) - { - return _cache.TryGetValue(expressionString, out lambda); - } - - public void Set(string expressionString, Lambda lambda) - { - _cache[expressionString] = lambda; - } - - public void Clear() => _cache.Clear(); -} - internal class ExpressionCache : IExpressionCache where TFunc : Delegate { - private readonly Dictionary _cache = new(); + private readonly ConcurrentDictionary _cache = new(); public bool TryGet(string expressionString, out TFunc func) { diff --git a/Casbin/Caching/ExpressionCachePool.cs b/Casbin/Caching/ExpressionCachePool.cs index 950da817..6f195b92 100644 --- a/Casbin/Caching/ExpressionCachePool.cs +++ b/Casbin/Caching/ExpressionCachePool.cs @@ -1,36 +1,12 @@ using System; -using System.Collections.Generic; -using DynamicExpresso; +using System.Collections.Concurrent; +using System.Threading; namespace Casbin.Caching; public class ExpressionCachePool : IExpressionCachePool { - private readonly Dictionary _cachePool = new(); - - public void SetLambda(string expression, Lambda lambda) - { - Type type = typeof(Lambda); - if (_cachePool.TryGetValue(type, out IExpressionCache cache) is false) - { - cache = new ExpressionCache(); - _cachePool[type] = cache; - } - var cacheImpl = (ExpressionCache) cache; - cacheImpl.Set(expression, lambda); - } - - public bool TryGetLambda(string expression, out Lambda lambda) - { - Type type = typeof(Lambda); - if (_cachePool.TryGetValue(type, out IExpressionCache cache) is false) - { - lambda = default; - return false; - } - var cacheImpl = (ExpressionCache) cache; - return cacheImpl.TryGet(expression, out lambda); - } + private ConcurrentDictionary _cachePool = new(); public void SetFunc(string expression, TFunc func) where TFunc : Delegate { @@ -53,15 +29,14 @@ public bool TryGetFunc(string expression, out TFunc func) where TFunc : D cache = new ExpressionCache(); _cachePool[type] = cache; } + var cacheImpl = (IExpressionCache)cache; return cacheImpl.TryGet(expression, out func); } public void Clear() { - foreach (IExpressionCache cache in _cachePool.Values) - { - cache?.Clear(); - } + ConcurrentDictionary cachePool = new ConcurrentDictionary(); + Interlocked.Exchange(ref _cachePool, cachePool); } } diff --git a/Casbin/Casbin.csproj b/Casbin/Casbin.csproj index 7f14b426..0b17105d 100644 --- a/Casbin/Casbin.csproj +++ b/Casbin/Casbin.csproj @@ -47,46 +47,46 @@ - + - + - + - + - + - + - + - + diff --git a/Casbin/Evaluation/ExpressionHandler.cs b/Casbin/Evaluation/ExpressionHandler.cs index 939cdcbd..8c0a5818 100644 --- a/Casbin/Evaluation/ExpressionHandler.cs +++ b/Casbin/Evaluation/ExpressionHandler.cs @@ -1,6 +1,5 @@ using System; using System.Collections.Generic; -using System.Linq; using System.Threading; using Casbin.Caching; using Casbin.Model; @@ -10,9 +9,13 @@ namespace Casbin.Evaluation; internal class ExpressionHandler : IExpressionHandler { - public IExpressionCachePool Cache { get; set; } = new ExpressionCachePool(); private readonly FunctionMap _functionMap = FunctionMap.LoadFunctionMap(); + private IExpressionCachePool _cachePool = new ExpressionCachePool(); +#if !NET452 + private readonly Interpreter _interpreter; +#else private Interpreter _interpreter; +#endif public ExpressionHandler() { @@ -24,6 +27,9 @@ public ExpressionHandler() public void SetFunction(string name, Delegate function) { +#if !NET452 + _interpreter.SetFunction(name, function); +#else List identifiers = new(); bool exist = false; foreach (var identifier in _interpreter.Identifiers) @@ -46,34 +52,42 @@ public void SetFunction(string name, Delegate function) interpreter.SetIdentifiers(identifiers); interpreter.SetFunction(name, function); Interlocked.Exchange(ref _interpreter, interpreter); - Cache.Clear(); +#endif + ExpressionCachePool cachePool = new ExpressionCachePool(); + Interlocked.Exchange(ref _cachePool, cachePool); } - public bool Invoke(in EnforceContext context, string expressionString, in TRequest request, in TPolicy policy) + public bool Invoke(in EnforceContext context, string expressionString, in TRequest request, + in TPolicy policy) where TRequest : IRequestValues where TPolicy : IPolicyValues { if (context.View.SupportGeneric is false) { - if (Cache.TryGetFunc>(expressionString, out var func)) + if (_cachePool.TryGetFunc>(expressionString, + out Func func)) { return func(request, policy); } + func = CompileExpression(in context, expressionString); - Cache.SetFunc(expressionString, func); + _cachePool.SetFunc(expressionString, func); return func(request, policy); } - if (Cache.TryGetFunc>(expressionString, out var genericFunc) is not false) + if (_cachePool.TryGetFunc>(expressionString, + out Func genericFunc) is not false) { return genericFunc(request, policy); } + genericFunc = CompileExpression(in context, expressionString); - Cache.SetFunc(expressionString, genericFunc); + _cachePool.SetFunc(expressionString, genericFunc); return genericFunc(request, policy); } - private Func CompileExpression(in EnforceContext context, string expressionString) + private Func CompileExpression(in EnforceContext context, + string expressionString) where TRequest : IRequestValues where TPolicy : IPolicyValues { @@ -89,6 +103,7 @@ private Interpreter CreateInterpreter() { interpreter.SetFunction(functionKeyValue.Key, functionKeyValue.Value); } + return interpreter; } } diff --git a/Casbin/Util/Utility.cs b/Casbin/Util/Utility.cs index 9ea4add8..0743e708 100644 --- a/Casbin/Util/Utility.cs +++ b/Casbin/Util/Utility.cs @@ -21,10 +21,12 @@ internal static bool SetEquals(List a, List b) { a = new List(); } + if (b == null) { b = new List(); } + if (a.Count != b.Count) { return false; @@ -40,6 +42,7 @@ internal static bool SetEquals(List a, List b) return false; } } + return true; } }