From bf1675eb69d8fa34993982383b63229e257266a0 Mon Sep 17 00:00:00 2001
From: Sean Young <sean.young@monax.io>
Date: Fri, 6 Apr 2018 11:26:49 +0100
Subject: [PATCH] Fix locking issues in state cache
In GetStorage() and get(), avoid potential concurrent duplicate backend
calls for the same entry by taking a write lock. This does increase
the time the cache is write locked.
Also avoid leaving the cache locked in early exit from the iterate
functions.
Signed-off-by: Sean Young <sean.young@monax.io>
---
account/state/state_cache.go | 48 +++++++++++++++++--------------
account/state/state_cache_test.go | 10 +++++++
2 files changed, 37 insertions(+), 21 deletions(-)
diff --git a/account/state/state_cache.go b/account/state/state_cache.go
index f9d85c97..fd7ad33c 100644
--- a/account/state/state_cache.go
+++ b/account/state/state_cache.go
@@ -103,6 +103,7 @@ func (cache *stateCache) IterateAccounts(consumer func(acm.Account) (stop bool))
cache.RLock()
for _, info := range cache.accounts {
if consumer(info.account) {
+ cache.RUnlock()
return true, nil
}
}
@@ -119,19 +120,20 @@ func (cache *stateCache) GetStorage(address acm.Address, key binary.Word256) (bi
accInfo.RLock()
value, ok := accInfo.storage[key]
accInfo.RUnlock()
- if ok {
- return value, nil
- } else {
- // Load from backend
- value, err := cache.backend.GetStorage(address, key)
- if err != nil {
- return binary.Zero256, err
- }
+ if !ok {
accInfo.Lock()
- accInfo.storage[key] = value
- accInfo.Unlock()
- return value, nil
+ defer accInfo.Unlock()
+ value, ok = accInfo.storage[key]
+ if !ok {
+ // Load from backend
+ value, err = cache.backend.GetStorage(address, key)
+ if err != nil {
+ return binary.Zero256, err
+ }
+ accInfo.storage[key] = value
+ }
}
+ return value, nil
}
// NOTE: Set value to zero to remove.
@@ -161,6 +163,7 @@ func (cache *stateCache) IterateStorage(address acm.Address,
// Try cache first for early exit
for key, value := range accInfo.storage {
if consumer(key, value) {
+ accInfo.RUnlock()
return true, nil
}
}
@@ -241,17 +244,20 @@ func (cache *stateCache) get(address acm.Address) (*accountInfo, error) {
accInfo := cache.accounts[address]
cache.RUnlock()
if accInfo == nil {
- account, err := cache.backend.GetAccount(address)
- if err != nil {
- return nil, err
- }
- accInfo = &accountInfo{
- account: account,
- storage: make(map[binary.Word256]binary.Word256),
- }
cache.Lock()
- cache.accounts[address] = accInfo
- cache.Unlock()
+ defer cache.Unlock()
+ accInfo = cache.accounts[address]
+ if accInfo == nil {
+ account, err := cache.backend.GetAccount(address)
+ if err != nil {
+ return nil, err
+ }
+ accInfo = &accountInfo{
+ account: account,
+ storage: make(map[binary.Word256]binary.Word256),
+ }
+ cache.accounts[address] = accInfo
+ }
}
return accInfo, nil
}
diff --git a/account/state/state_cache_test.go b/account/state/state_cache_test.go
index de9683b2..80801d8c 100644
--- a/account/state/state_cache_test.go
+++ b/account/state/state_cache_test.go
@@ -78,6 +78,16 @@ func TestStateCache_UpdateAccount(t *testing.T) {
}
func TestStateCache_RemoveAccount(t *testing.T) {
+ // Build backend states for read and write
+ readBackend := testAccounts()
+ cache := NewCache(readBackend)
+
+ acc := readBackend.Accounts[addressOf("acc1")]
+ err := cache.RemoveAccount(acc.Address())
+ require.NoError(t, err)
+
+ dead, err := cache.GetAccount(acc.Address())
+ assert.Nil(t, dead, err)
}
func TestStateCache_GetStorage(t *testing.T) {
--
GitLab