Merge pull request #3985 from aduffeck/fix-concurrency-issue

Fix concurrent access to the lock map
cs3org · Jun 16, 2023 · b30fdde · b30fdde
2 parents 71e9489 + a2fec67
commit b30fdde
Show file tree

Hide file tree

Showing 4 changed files with 16 additions and 39 deletions.
diff --git a/changelog/unreleased/jsoncs3-lock-congestion.md b/changelog/unreleased/jsoncs3-lock-congestion.md
@@ -3,4 +3,5 @@ Bugfix: Reduce jsoncs3 lock congestion
 We changed the locking strategy in the jsoncs3 share manager to cause less lock
  congestion increasing the performance in certain scenarios.
 
+https://github.com/cs3org/reva/pull/3985
 https://github.com/cs3org/reva/pull/3964
diff --git a/pkg/share/manager/jsoncs3/providercache/providercache.go b/pkg/share/manager/jsoncs3/providercache/providercache.go
@@ -43,8 +43,7 @@ const tracerName = "providercache"
 
 // Cache holds share information structured by provider and space
 type Cache struct {
-	lockMapLock sync.Mutex
-	lockMap     map[string]*sync.Mutex
+	lockMap sync.Map
 
 	Providers map[string]*Spaces
 
@@ -106,16 +105,9 @@ func (s *Shares) UnmarshalJSON(data []byte) error {
 
 // LockSpace locks the cache for a given space and returns an unlock function
 func (c *Cache) LockSpace(spaceID string) func() {
-	lock := c.lockMap[spaceID]
-	if lock == nil {
-		c.lockMapLock.Lock()
-		lock = c.lockMap[spaceID]
-		if lock == nil {
-			c.lockMap[spaceID] = &sync.Mutex{}
-			lock = c.lockMap[spaceID]
-		}
-		c.lockMapLock.Unlock()
-	}
+	v, _ := c.lockMap.LoadOrStore(spaceID, &sync.Mutex{})
+	lock := v.(*sync.Mutex)
+
 	lock.Lock()
 	return func() { lock.Unlock() }
 }
@@ -126,7 +118,7 @@ func New(s metadata.Storage, ttl time.Duration) Cache {
 		Providers: map[string]*Spaces{},
 		storage:   s,
 		ttl:       ttl,
-		lockMap:   map[string]*sync.Mutex{},
+		lockMap:   sync.Map{},
 	}
 }
 

diff --git a/pkg/share/manager/jsoncs3/receivedsharecache/receivedsharecache.go b/pkg/share/manager/jsoncs3/receivedsharecache/receivedsharecache.go
@@ -44,8 +44,7 @@ const tracerName = "receivedsharecache"
 // It functions as an in-memory cache with a persistence layer
 // The storage is sharded by user
 type Cache struct {
-	lockMapLock sync.Mutex
-	lockMap     map[string]*sync.Mutex
+	lockMap sync.Map
 
 	ReceivedSpaces map[string]*Spaces
 
@@ -79,21 +78,14 @@ func New(s metadata.Storage, ttl time.Duration) Cache {
 		ReceivedSpaces: map[string]*Spaces{},
 		storage:        s,
 		ttl:            ttl,
-		lockMap:        map[string]*sync.Mutex{},
+		lockMap:        sync.Map{},
 	}
 }
 
 func (c *Cache) lockUser(userID string) func() {
-	lock := c.lockMap[userID]
-	if lock == nil {
-		c.lockMapLock.Lock()
-		lock = c.lockMap[userID]
-		if lock == nil {
-			c.lockMap[userID] = &sync.Mutex{}
-			lock = c.lockMap[userID]
-		}
-		c.lockMapLock.Unlock()
-	}
+	v, _ := c.lockMap.LoadOrStore(userID, &sync.Mutex{})
+	lock := v.(*sync.Mutex)
+
 	lock.Lock()
 	return func() { lock.Unlock() }
 }

diff --git a/pkg/share/manager/jsoncs3/sharecache/sharecache.go b/pkg/share/manager/jsoncs3/sharecache/sharecache.go
@@ -43,8 +43,7 @@ const tracerName = "sharecache"
 // It functions as an in-memory cache with a persistence layer
 // The storage is sharded by user/group
 type Cache struct {
-	lockMapLock sync.Mutex
-	lockMap     map[string]*sync.Mutex
+	lockMap sync.Map
 
 	UserShares map[string]*UserShareCache
 
@@ -69,16 +68,9 @@ type SpaceShareIDs struct {
 }
 
 func (c *Cache) lockUser(userID string) func() {
-	lock := c.lockMap[userID]
-	if lock == nil {
-		c.lockMapLock.Lock()
-		lock = c.lockMap[userID]
-		if lock == nil {
-			c.lockMap[userID] = &sync.Mutex{}
-			lock = c.lockMap[userID]
-		}
-		c.lockMapLock.Unlock()
-	}
+	v, _ := c.lockMap.LoadOrStore(userID, &sync.Mutex{})
+	lock := v.(*sync.Mutex)
+
 	lock.Lock()
 	return func() { lock.Unlock() }
 }
@@ -91,7 +83,7 @@ func New(s metadata.Storage, namespace, filename string, ttl time.Duration) Cach
 		namespace:  namespace,
 		filename:   filename,
 		ttl:        ttl,
-		lockMap:    map[string]*sync.Mutex{},
+		lockMap:    sync.Map{},
 	}
 }