Skip to content

Commit

Permalink
fix: concurrent access issues with HashSet<string> (#361)
Browse files Browse the repository at this point in the history
  • Loading branch information
seanamorosoamtote authored Aug 3, 2024
1 parent 6d3cb37 commit 6863c41
Show file tree
Hide file tree
Showing 5 changed files with 79 additions and 3 deletions.
5 changes: 5 additions & 0 deletions Casbin.UnitTests/Fixtures/TestModelFixture.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ public class TestModelFixture
public static readonly string BasicWithoutUserPolicyText = ReadTestFile("basic_without_users_policy.csv");
public static readonly string BasicWithRootModelText = ReadTestFile("basic_with_root_model.conf");

public static readonly string SaaRbacModelText = ReadTestFile("saa_model.conf");
public static readonly string SaaRbacPolicyText = ReadTestFile("saa_rbac_policy.csv");

public static readonly string RbacPolicyText = ReadTestFile("rbac_policy.csv");
public static readonly string RbacCommentText = ReadTestFile("rbac_comment.conf");
public static readonly string RbacInOperatorModelText = ReadTestFile("rbac_in_operator_model.conf");
Expand Down Expand Up @@ -123,6 +126,8 @@ public static IModel GetNewPriorityExplicitTestModel() =>
public static IModel GetNewPriorityExplicitDenyOverrideModel() => GetNewTestModel(PriorityExplicitDenyOverrideModelText,
PriorityExplicitDenyOverridePolicyText);

public static IModel GetNewSaaRbacTestModel() => GetNewTestModel(SaaRbacModelText, SaaRbacPolicyText);

public static IModel GetNewRbacTestModel() => GetNewTestModel(RbacModelText, RbacPolicyText);

public static IModel GetNewRbacWithDenyTestModel() => GetNewTestModel(RbacWithDenyModelText, RbacWithDenyPolicyText);
Expand Down
41 changes: 41 additions & 0 deletions Casbin.UnitTests/ModelTests/ManagementApiTest.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.Collections.Generic;
using System.Linq;
using System.Threading.Tasks;
using Casbin.Model;
using Casbin.UnitTests.Extensions;
Expand Down Expand Up @@ -323,6 +324,46 @@ public async Task TestModifyPolicyAsync()
Assert.False(res);
}

private static List<List<string>> GenerateGroupingRules(int numberOfItems)
{
var groupingRules = new List<List<string>>();

for (int i = 1; i <= numberOfItems; i++)
{
string parent = $"Parent{i}";
string child = $"Child{i}";
groupingRules.Add(new List<string> { parent, child });
}

return groupingRules;
}

[Fact]
public void TestConcurrentModifyGroupingPolicy()
{
Enforcer e = new(TestModelFixture.GetNewSaaRbacTestModel());
e.BuildRoleLinks();

// Arrange
var policiesBeforeAct = e.GetNamedGroupingPolicy(PermConstants.GroupingPolicyType2).ToArray();
Assert.Empty(policiesBeforeAct);

var groupingRules = GenerateGroupingRules(1000);

Task.WaitAll(groupingRules.Select(rule => Task.Run(() =>
{
bool result = e.AddNamedGroupingPolicies(PermConstants.GroupingPolicyType2, new List<List<string>> { rule });
Assert.True(result);
})).ToArray());

// Assert
var policiesAfterAct = e.GetNamedGroupingPolicy(PermConstants.GroupingPolicyType2).ToArray();
foreach (var policy in groupingRules)
{
Assert.Contains(policy, policiesAfterAct);
}
}

[Fact]
public void TestModifyGroupingPolicy()
{
Expand Down
17 changes: 17 additions & 0 deletions Casbin.UnitTests/examples/saa_model.conf
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
[request_definition]
# request includes subject, domain parent of obj, object requesting access for, action requested
r = sub, dom, obj, act

[policy_definition]
p = sub, dom, act

[role_definition]
g = _, _
g2 = _, _
g3 = _, _

[policy_effect]
e = some(where (p.eft == allow))

[matchers]
m = g(r.sub, p.sub) && (g2(r.dom, p.dom) || r.obj == p.dom) && g3(r.act, p.act)
3 changes: 3 additions & 0 deletions Casbin.UnitTests/examples/saa_rbac_policy.csv
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# sample policy, not needed for test
p, tellers, banks/1, wager-editor

16 changes: 13 additions & 3 deletions Casbin/Model/DefaultPolicyStore.Node.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,17 @@ public IReadOnlyList<IPolicyValues> SetPolicy(List<IPolicyValues> valuesList) =>
Interlocked.Exchange(ref Policy, valuesList);

public bool ContainsPolicy(IPolicyValues values)
=> PolicyTextSet.Contains(values.ToText());
{
Lock.EnterReadLock();
try
{
return PolicyTextSet.Contains(values.ToText());
}
finally
{
Lock.ExitReadLock();
}
}

public bool ValidatePolicy(IPolicyValues values)
{
Expand Down Expand Up @@ -67,13 +77,13 @@ public bool TryAddPolicy(IPolicyValues values)
try
{
Policy.Add(values);
PolicyTextSet.Add(values.ToText());
}
finally
{
Lock.ExitWriteLock();
}

PolicyTextSet.Add(values.ToText());
return true;
}

Expand Down Expand Up @@ -244,13 +254,13 @@ bool LastLessOrEqualPriority(IPolicyValues v)
{
int lastIndex = Policy.FindLastIndex(LastLessOrEqualPriority);
Policy.Insert(lastIndex + 1, values);
PolicyTextSet.Add(values.ToText());
}
finally
{
Lock.ExitWriteLock();
}

PolicyTextSet.Add(values.ToText());
return true;
}

Expand Down

0 comments on commit 6863c41

Please # to comment.