Skip to content

Commit

Permalink
Fixes cache data race bug.
Browse files Browse the repository at this point in the history
This is achieved in two ways: First, finish making the current height "unreadable". Second, make the lowest height in the cache also "unreadable". We kept the amount of accessible previous blocks the same by increasing the cache's capacity by one.

The lowest height's situation is a hypothetical issue that could come up if an iterator is being generated for the lowest height in the cache when a Commit starts, given that it evicts the lowest height. By only returning information (aka "Reading from") heights that cannot be written to, we eliminate this class of issue without needing locks or other synchronization mechanisms.
  • Loading branch information
iajrz committed Oct 13, 2021
1 parent 75fceeb commit 1200b29
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 13 deletions.
2 changes: 1 addition & 1 deletion app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import (
)

const (
AppVersion = "BETA-0.6.3.8"
AppVersion = "BETA-0.6.3.9"
)

// NewPocketCoreApp is a constructor function for PocketCoreApp
Expand Down
17 changes: 6 additions & 11 deletions store/rootmulti/heightcache/memorycache.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ func (m *MemoryCache) InitializeStoreCache(height int64) error {
return nil
}

func (m MemoryCache) isValid(height int64) bool {
if height > m.current.height-m.capacity {
func (m MemoryCache) isHeightSafeToRead(height int64) bool {
if height != m.current.height && height > m.current.height-(1+m.capacity) {
for _, v := range m.pastHeights {
if v.height == height {
return true
Expand All @@ -48,7 +48,7 @@ func (m MemoryCache) isValid(height int64) bool {
}

func (m MemoryCache) Get(height int64, key []byte) ([]byte, error) {
if height != m.current.height && m.isValid(height) {
if m.isHeightSafeToRead(height) {
for i := range m.pastHeights {
if m.pastHeights[i].height == height {
return []byte(m.pastHeights[i].data[string(key)]), nil
Expand All @@ -72,7 +72,7 @@ func (m MemoryCache) Remove(key []byte) error {
}

func (m MemoryCache) Iterator(height int64, start, end []byte) (types.Iterator, error) {
if height != m.current.height && m.isValid(height) {
if m.isHeightSafeToRead(height) {
for _, v := range m.pastHeights {
if v.height == height {
return NewMemoryHeightIterator(v.data, string(start), string(end), v.orderedKeys, true), nil
Expand All @@ -83,19 +83,14 @@ func (m MemoryCache) Iterator(height int64, start, end []byte) (types.Iterator,
}

func (m MemoryCache) ReverseIterator(height int64, start, end []byte) (types.Iterator, error) {
if !m.isValid(height) {
return nil, errors.New("invalid height for iterator")
}
if height == m.current.height {
return NewMemoryHeightIterator(m.current.data, string(start), string(end), []string{}, false), nil
} else {
if m.isHeightSafeToRead(height) {
for _, v := range m.pastHeights {
if v.height == height {
return NewMemoryHeightIterator(v.data, string(start), string(end), v.orderedKeys, false), nil
}
}
}
return NewMemoryHeightIterator(map[string]string{}, string(start), string(end), []string{}, false), nil
return nil, errors.New("invalid height for iterator")
}

func (m MemoryCache) Commit(height int64) {
Expand Down
4 changes: 3 additions & 1 deletion store/rootmulti/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,12 @@ func (rs *Store) CopyStore() *types.Store {
var _ types.CommitMultiStore = (*Store)(nil)
var _ types.Queryable = (*Store)(nil)

const MemoryCacheCapacity = 26

func NewStore(db dbm.DB, cache bool) *Store {
var multiStoreCache types.MultiStoreCache
if cache {
multiStoreCache = heightcache.NewMultiStoreMemoryCache(25)
multiStoreCache = heightcache.NewMultiStoreMemoryCache(MemoryCacheCapacity)
} else {
multiStoreCache = heightcache.NewMultiStoreInvalidCache()
}
Expand Down

0 comments on commit 1200b29

Please sign in to comment.