Repository: casbin/casbin (go)

Issue:
[Bug] Data race in RoleManagerImpl.HasLink() breaks Enforcer when calling Enforce()
## **Describe the bug**

Setup: 
* Initialized Casbin Enforcer or Synced Casbin Enforcer
* `AddNamedMatchingFunc()` used (e.g. keyMatch2, keyMatch4)

The bug is reproduced when calling `Enforce()` with the same parameters concurrently.

It boils down to this path being executed concurrently:
[GenerateGFunction()](https://github.com/casbin/casbin/blob/master/enforcer.go#L595)->[HasLink()](https://github.com/casbin/casbin/blob/master/util/builtin_operators.go#L420)->[defer rm.removeRole()](https://github.com/casbin/casbin/blob/master/rbac/default-role-manager/role_manager.go#L356).

Race explanation inside `HasLink()` function with goroutines A and B:
* Enforce A: Enter `HasLink()`. Role does not exist, is added to map temporarily && `defer rm.removeRole()` called
* Enforce A: `hasLinkHelper` iterates over roles via `sync.Map.Range()` recursively
* Enforce B: Temp role exists, fetch it.
* Enforce A: Exit function, `removeRole()` called.
* Enforce B: `hasLinkHelper` iterates over roles but the temp role no longer exists
* Enforce A: `allow: true`
* Enforce B: `allow: false`

Furthermore once this happens, the calculation is cached in `GenerateGFunction` and also in [getAndStoreMatcherExpression()](https://github.com/casbin/casbin/blob/master/enforcer.go#L655), all further `Enforce()` calls for that set of parameters will incorrectly return a cached `false` response.

## **To Reproduce**

This behaviour does not occur all the time, but by disabling cache inside `GenerateGFunction` it is possible to reproduce it reliably:
1. Clone https://github.com/Metrika-Inc/casbin-race/
3. Run the test: `make test`
    * rerun if not reproduced on first time
4. Observe the panic due to Enforce() returning false [in the test](https://github.com/Metrika-Inc/casbin-race/blob/master/race_test.go#L69)

Check README in the [Metrika-Inc/casbin-race](https://github.com/Metrika-Inc/casbin-race) repository for more details.

## **Expected behavior**

Enforcer always returns true when correct policies are in place.

## Actual behavior
Enforcer sometimes returns false even when correct policies are in place and it may cache the result to continue returning false incorrectly.

## Environment
 - OS: macOS and Linux

## Possible solutions
It seems that feasible solutions may be having a mutex on `HasLink()` or not storing temporary roles in a shared map and having a temporary map defined inside the scope of the function. 

Pull request:
feat: prevent race condition in HasLink with temporary roles
close #1318

Add `mutex `protection to `HasLink `operations in both `RoleManagerImpl `and `ConditionalRoleManager`

Patch:
diff --git a/rbac/default-role-manager/role_manager.go b/rbac/default-role-manager/role_manager.go
index a6ae869..ee1ee3f 100644
--- a/rbac/default-role-manager/role_manager.go
+++ b/rbac/default-role-manager/role_manager.go
@@ -203,6 +203,7 @@ type RoleManagerImpl struct {
 	domainMatchingFunc rbac.MatchingFunc
 	logger             log.Logger
 	matchingFuncCache  *util.SyncLRUCache
+	mutex              sync.Mutex
 }
 
 // NewRoleManagerImpl is the constructor for creating an instance of the
@@ -349,6 +350,10 @@ func (rm *RoleManagerImpl) HasLink(name1 string, name2 string, domains ...string
 		return true, nil
 	}
 
+	// Lock to prevent race conditions between getRole and removeRole
+	rm.mutex.Lock()
+	defer rm.mutex.Unlock()
+
 	user, userCreated := rm.getRole(name1)
 	role, roleCreated := rm.getRole(name2)
 
@@ -756,6 +761,10 @@ func (crm *ConditionalRoleManager) HasLink(name1 string, name2 string, domains .
 		return true, nil
 	}
 
+	// Lock to prevent race conditions between getRole and removeRole
+	crm.mutex.Lock()
+	defer crm.mutex.Unlock()
+
 	user, userCreated := crm.getRole(name1)
 	role, roleCreated := crm.getRole(name2)
 


Test patch:
diff --git a/rbac/default-role-manager/role_manager_test.go b/rbac/default-role-manager/role_manager_test.go
index 5af35c6..142f1eb 100644
--- a/rbac/default-role-manager/role_manager_test.go
+++ b/rbac/default-role-manager/role_manager_test.go
@@ -16,6 +16,8 @@ package defaultrolemanager
 
 import (
 	"fmt"
+	"sync"
+	"sync/atomic"
 	"testing"
 
 	"github.com/casbin/casbin/v2/rbac"
@@ -386,3 +388,46 @@ func TestMaxHierarchyLevel(t *testing.T) {
 	testRole(t, rm, "level1", "level2", true)
 	testRole(t, rm, "level1", "level3", true)
 }
+
+// TestConcurrentHasLink tests concurrent HasLink calls for race conditions.
+// This test verifies that concurrent HasLink calls with matching functions
+// don't produce inconsistent results due to temporary role creation/deletion races.
+// Regression test for issue #1318.
+func TestConcurrentHasLink(t *testing.T) {
+	rm := NewRoleManager(10)
+	rm.AddMatchingFunc("keyMatch2", util.KeyMatch2)
+
+	_ = rm.AddLink("alice", "admin")
+	_ = rm.AddLink("admin", "/data/*")
+
+	expected, _ := rm.HasLink("alice", "/data/123")
+
+	const numGoroutines = 20
+	const numIterations = 100
+
+	var inconsistencies int64
+	var wg sync.WaitGroup
+
+	for i := 0; i < numGoroutines; i++ {
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+			for j := 0; j < numIterations; j++ {
+				result, err := rm.HasLink("alice", "/data/123")
+				if err != nil {
+					t.Errorf("HasLink failed: %v", err)
+					return
+				} else if result != expected {
+					atomic.AddInt64(&inconsistencies, 1)
+				}
+			}
+		}()
+	}
+
+	wg.Wait()
+
+	if inconsistencies > 0 {
+		t.Errorf("Found %d inconsistencies in %d total operations",
+			inconsistencies, numGoroutines*numIterations)
+	}
+}
diff --git a/role_manager_b_test.go b/role_manager_b_test.go
index 32da7f1..1f38643 100644
--- a/role_manager_b_test.go
+++ b/role_manager_b_test.go
@@ -178,3 +178,18 @@ func BenchmarkHasLinkWithPatternAndDomainPatternLarge(b *testing.B) {
 		_, _ = rm.HasLink("staffUser1001", "staff001", "/orgs/1/sites/site001")
 	}
 }
+
+// BenchmarkConcurrentHasLinkWithMatching benchmarks concurrent HasLink performance with matching functions.
+// Performance test for concurrent access with temporary role creation.
+func BenchmarkConcurrentHasLinkWithMatching(b *testing.B) {
+	e, _ := NewEnforcer("examples/rbac_with_pattern_model.conf", "examples/rbac_with_pattern_policy.csv")
+	e.AddNamedMatchingFunc("g2", "keyMatch2", util.KeyMatch2)
+	rm := e.GetRoleManager()
+
+	b.ResetTimer()
+	b.RunParallel(func(pb *testing.PB) {
+		for pb.Next() {
+			_, _ = rm.HasLink("alice", "/book/123")
+		}
+	})
+}