Problem: concurrent map access in multistore

.github/workflows/build.yml

@@ -149,6 +149,7 @@ jobs:
         with:
           name: cronos
           extraPullNames: dapp
+          installCommand: "nix-env --quiet -j8 -iA cachix -f https://cachix.org/api/v1/install/1.6.1"
           signingKey: "${{ secrets.CACHIX_SIGNING_KEY }}"
       - name: test contracts
         if: steps.changed-files.outputs.any_changed == 'true'

.github/workflows/test.yml

@@ -44,6 +44,7 @@ jobs:
         with:
           name: cronos
           extraPullNames: dapp
+          installCommand: "nix-env --quiet -j8 -iA cachix -f https://cachix.org/api/v1/install/1.6.1"
           # github don't pass secrets for pull request from fork repos,
           # in that case the push is disabled naturally.
           signingKey: "${{ secrets.CACHIX_SIGNING_KEY }}"
@@ -101,6 +102,7 @@ jobs:
         with:
           name: cronos
           extraPullNames: dapp
+          installCommand: "nix-env --quiet -j8 -iA cachix -f https://cachix.org/api/v1/install/1.6.1"
           # github don't pass secrets for pull request from fork repos,
           # in that case the push is disabled naturally.
           signingKey: "${{ secrets.CACHIX_SIGNING_KEY }}"

CHANGELOG.md

@@ -1,5 +1,10 @@
 # Changelog
 
+## UNRELEASED
+
+- [#1299](https://github.com/crypto-org-chain/cronos/pull/1299) Avoid concurrent map access in multistore on grpc call.
+
+
 *January 5, 2024*
 
 ## v1.1.0-rc2

store/rootmulti/store.go

@@ -6,6 +6,7 @@
 	"math"
 	"sort"
 	"strings"
+	"sync"
 
 	"cosmossdk.io/errors"
 	dbm "github.com/cometbft/cometbft-db"
@@ -50,6 +51,7 @@
 	sdk46Compact bool
 	// it's more efficient to export snapshot versions, we can filter out the non-snapshot versions
 	supportExportNonSnapshotVersion bool
+	mtx                             sync.RWMutex
 }
 
 func NewStore(dir string, logger log.Logger, sdk46Compact bool, supportExportNonSnapshotVersion bool) *Store {
@@ -69,19 +71,18 @@
 // flush writes all the pending change sets to memiavl tree.
 func (rs *Store) flush() error {
 	var changeSets []*memiavl.NamedChangeSet
-	for key := range rs.stores {
+	for key, store := range rs.stores {
 		// it'll unwrap the inter-block cache
-		store := rs.GetCommitKVStore(key)
 		if memiavlStore, ok := store.(*memiavlstore.Store); ok {
 			cs := memiavlStore.PopChangeSet()
 			if len(cs.Pairs) > 0 {
 				changeSets = append(changeSets, &memiavl.NamedChangeSet{
 					Name:      key.Name(),
 					Changeset: cs,
 				})
 			}
 		}
 	}
 	sort.SliceStable(changeSets, func(i, j int) bool {
 		return changeSets[i].Name < changeSets[j].Name
 	})
@@ -93,7 +94,10 @@
 //
 // Implements interface Committer.
 func (rs *Store) WorkingHash() []byte {
-	if err := rs.flush(); err != nil {
+	rs.mtx.RLock()
+	err := rs.flush()
+	rs.mtx.RUnlock()
+	if err != nil {
 		panic(err)
 	}
 	commitInfo := convertCommitInfo(rs.db.WorkingCommitInfo())
@@ -105,29 +109,36 @@
 
 // Implements interface Committer
 func (rs *Store) Commit() types.CommitID {
-	if err := rs.flush(); err != nil {
+	rs.mtx.RLock()
+	err := rs.flush()
+	rs.mtx.RUnlock()
+	if err != nil {
 		panic(err)
 	}
 
-	for _, store := range rs.stores {
+	iavlStoreKeys := make([]types.StoreKey, 0)
+	rs.mtx.RLock()
+	for key, store := range rs.stores {
 		if store.GetStoreType() != types.StoreTypeIAVL {
 			_ = store.Commit()
+		} else {
+			iavlStoreKeys = append(iavlStoreKeys, key)
 		}
 	}
+	rs.mtx.RUnlock()
 
-	_, err := rs.db.Commit()
+	_, err = rs.db.Commit()
 	if err != nil {
 		panic(err)
 	}
 
 	// the underlying memiavl tree might be reloaded, reload the store as well.
-	for key := range rs.stores {
-		store := rs.stores[key]
-		if store.GetStoreType() == types.StoreTypeIAVL {
-			rs.stores[key], err = rs.loadCommitStoreFromParams(rs.db, key, rs.storesParams[key])
-			if err != nil {
-				panic(fmt.Errorf("inconsistent store map, store %s not found", key.Name()))
-			}
+	for _, key := range iavlStoreKeys {
+		rs.mtx.Lock()
+		rs.stores[key], err = rs.loadCommitStoreFromParams(rs.db, key, rs.storesParams[key])
+		rs.mtx.Unlock()
+		if err != nil {
+			panic(fmt.Errorf("inconsistent store map, store %s not found", key.Name()))
 		}
 	}
 
@@ -182,6 +193,7 @@
 // Implements interface MultiStore
 func (rs *Store) CacheMultiStore() types.CacheMultiStore {
 	stores := make(map[types.StoreKey]types.CacheWrapper)
+	rs.mtx.RLock()
 	for k, v := range rs.stores {
 		store := types.KVStore(v)
 		// Wire the listenkv.Store to allow listeners to observe the writes from the cache store,
@@ -191,6 +203,7 @@
 		}
 		stores[k] = store
 	}
+	rs.mtx.RUnlock()
 	return cachemulti.NewStore(nil, stores, rs.keysByName, nil, nil, nil)
 }
 
@@ -211,11 +224,13 @@
 	stores := make(map[types.StoreKey]types.CacheWrapper)
 
 	// add the transient/mem stores registered in current app.
+	rs.mtx.RLock()
 	for k, store := range rs.stores {
 		if store.GetStoreType() != types.StoreTypeIAVL {
 			stores[k] = store
 		}
 	}
+	rs.mtx.RUnlock()
 
 	// add all the iavl stores at the target version.
 	for _, tree := range db.Trees() {
@@ -227,11 +242,15 @@
 
 // Implements interface MultiStore
 func (rs *Store) GetStore(key types.StoreKey) types.Store {
+	rs.mtx.RLock()
+	defer rs.mtx.RUnlock()
 	return rs.stores[key]
 }
 
 // Implements interface MultiStore
 func (rs *Store) GetKVStore(key types.StoreKey) types.KVStore {
+	rs.mtx.RLock()
+	defer rs.mtx.RUnlock()
 	return rs.stores[key]
 }
 
@@ -287,6 +306,8 @@
 
 // Implements interface CommitMultiStore
 func (rs *Store) GetCommitKVStore(key types.StoreKey) types.CommitKVStore {
+	rs.mtx.RLock()
+	defer rs.mtx.RUnlock()
 	return rs.stores[key]
 }
 
@@ -361,7 +382,9 @@
 	}
 
 	rs.db = db
+	rs.mtx.Lock()
 	rs.stores = newStores
+	rs.mtx.Unlock()
 	// to keep the root hash compatible with cosmos-sdk 0.46
 	if db.Version() != 0 {
 		rs.lastCommitInfo = convertCommitInfo(db.LastCommitInfo())