Repository: casbin/casbin (go)

Issue:
[Bug] SyncedCachedEnforcer: RUnlock of unlocked RWMutex
**Want to prioritize this issue? Try:**

[![issuehunt-to-marktext](https://github.com/BoostIO/issuehunt-materials/raw/master/v1/issuehunt-button-v1.svg)](https://issuehunt.io/r/casbin/casbin)

------

**Describe the bug**
Fatal error during runtime.
```
api[586577]: fatal error: sync: RUnlock of unlocked RWMutex
api[586577]: goroutine 564104007 [running]:
api[586577]: sync.fatal({0x28eb0d0?, 0x31db0e67170?})
api[586577]:         runtime/panic.go:1031 +0x1e
api[586577]: sync.(*RWMutex).rUnlockSlow(0xc00fd992a8, 0xabc4a2d0?)
api[586577]:         sync/rwmutex.go:129 +0x39
api[586577]: sync.(*RWMutex).RUnlock(...)
api[586577]:         sync/rwmutex.go:119
api[586577]: github.com/casbin/casbin/v2/persist/cache.(*SyncCache).Get(0xc00fd992a0, {0xc7acc054a0, 0x5d})
api[586577]:         github.com/casbin/casbin/v2@v2.66.1/persist/cache/cache_sync.go:45 +0x145
api[586577]: github.com/casbin/casbin/v2.(*SyncedCachedEnforcer).getCachedResult(...)
api[586577]:         github.com/casbin/casbin/v2@v2.66.1/enforcer_cached_synced.go:123
api[586577]: github.com/casbin/casbin/v2.(*SyncedCachedEnforcer).Enforce(0xc0f71a3f50, {0x2e01a593c40, 0x4, 0x4})
api[586577]:         github.com/casbin/casbin/v2@v2.66.1/enforcer_cached_synced.go:70 +0x85
```

**To Reproduce**
Steps to reproduce the behavior:
1. Use SyncedCachedEnforcer

**Expected behavior**
No crash


Pull request description:
feat: don't crash if Clear gets called while holding a lock
In a highly concurrent situation, Clear can be called while a mutex is held, which can cause issues when trying to unlock the mutex.

Fixes #1228 

Patch:
diff --git a/persist/cache/cache_sync.go b/persist/cache/cache_sync.go
index d083a03..816e12d 100644
--- a/persist/cache/cache_sync.go
+++ b/persist/cache/cache_sync.go
@@ -29,9 +29,9 @@ func (c *SyncCache) Set(key string, value bool, extra ...interface{}) error {
 	if len(extra) > 0 {
 		ttl = extra[0].(time.Duration)
 	}
-	(*c).Lock()
-	defer (*c).Unlock()
-	(*c).cache[key] = cacheItem{
+	c.Lock()
+	defer c.Unlock()
+	c.cache[key] = cacheItem{
 		value:     value,
 		expiresAt: time.Now().Add(ttl),
 		ttl:       ttl,
@@ -40,16 +40,16 @@ func (c *SyncCache) Set(key string, value bool, extra ...interface{}) error {
 }
 
 func (c *SyncCache) Get(key string) (bool, error) {
-	(*c).RLock()
-	res, ok := (*c).cache[key]
-	(*c).RUnlock()
+	c.RLock()
+	res, ok := c.cache[key]
+	c.RUnlock()
 	if !ok {
 		return false, ErrNoSuchKey
 	} else {
 		if res.ttl > 0 && time.Now().After(res.expiresAt) {
-			(*c).Lock()
-			defer (*c).Unlock()
-			delete((*c).cache, key)
+			c.Lock()
+			defer c.Unlock()
+			delete(c.cache, key)
 			return false, ErrNoSuchKey
 		}
 		return res.value, nil
@@ -57,24 +57,23 @@ func (c *SyncCache) Get(key string) (bool, error) {
 }
 
 func (c *SyncCache) Delete(key string) error {
-	(*c).RLock()
-	_, ok := (*c).cache[key]
-	(*c).RUnlock()
+	c.RLock()
+	_, ok := c.cache[key]
+	c.RUnlock()
 	if !ok {
 		return ErrNoSuchKey
 	} else {
-		(*c).Lock()
-		defer (*c).Unlock()
-		delete((*c).cache, key)
+		c.Lock()
+		defer c.Unlock()
+		delete(c.cache, key)
 		return nil
 	}
 }
 
 func (c *SyncCache) Clear() error {
-	*c = SyncCache{
-		make(DefaultCache),
-		sync.RWMutex{},
-	}
+	c.Lock()
+	c.cache = make(DefaultCache)
+	c.Unlock()
 	return nil
 }
 


Test patch:
diff --git a/enforcer_cached_synced_test.go b/enforcer_cached_synced_test.go
index 9738d8c..06c84c4 100644
--- a/enforcer_cached_synced_test.go
+++ b/enforcer_cached_synced_test.go
@@ -17,6 +17,7 @@ package casbin
 import (
 	"sync"
 	"testing"
+	"time"
 )
 
 func testSyncEnforceCache(t *testing.T, e *SyncedCachedEnforcer, sub string, obj interface{}, act string, res bool) {
@@ -28,6 +29,7 @@ func testSyncEnforceCache(t *testing.T, e *SyncedCachedEnforcer, sub string, obj
 
 func TestSyncCache(t *testing.T) {
 	e, _ := NewSyncedCachedEnforcer("examples/basic_model.conf", "examples/basic_policy.csv")
+	e.expireTime = time.Millisecond
 	// The cache is enabled by default for NewCachedEnforcer.
 	g := sync.WaitGroup{}
 	goThread := 1000
@@ -36,6 +38,9 @@ func TestSyncCache(t *testing.T) {
 		go func() {
 			_, _ = e.AddPolicy("alice", "data2", "read")
 			testSyncEnforceCache(t, e, "alice", "data2", "read", true)
+			if e.InvalidateCache() != nil {
+				panic("never reached")
+			}
 			g.Done()
 		}()
 	}
@@ -43,6 +48,9 @@ func TestSyncCache(t *testing.T) {
 	_, _ = e.RemovePolicy("alice", "data2", "read")
 
 	testSyncEnforceCache(t, e, "alice", "data1", "read", true)
+	time.Sleep(time.Millisecond * 2) // coverage for expire
+	testSyncEnforceCache(t, e, "alice", "data1", "read", true)
+
 	testSyncEnforceCache(t, e, "alice", "data1", "write", false)
 	testSyncEnforceCache(t, e, "alice", "data2", "read", false)
 	testSyncEnforceCache(t, e, "alice", "data2", "write", false)