fix race condition on address.String (#8997)
This commit is contained in:
parent
1fa2c22d8a
commit
0c824f11e9
|
@ -78,11 +78,11 @@ const (
|
|||
var (
|
||||
// AccAddress.String() is expensive and if unoptimized dominantly showed up in profiles,
|
||||
// yet has no mechanisms to trivially cache the result given that AccAddress is a []byte type.
|
||||
accAddrMu sync.RWMutex
|
||||
accAddrMu sync.Mutex
|
||||
accAddrCache *simplelru.LRU
|
||||
consAddrMu sync.RWMutex
|
||||
consAddrMu sync.Mutex
|
||||
consAddrCache *simplelru.LRU
|
||||
valAddrMu sync.RWMutex
|
||||
valAddrMu sync.Mutex
|
||||
valAddrCache *simplelru.LRU
|
||||
)
|
||||
|
||||
|
@ -268,13 +268,13 @@ func (aa AccAddress) String() string {
|
|||
}
|
||||
|
||||
var key = conv.UnsafeBytesToStr(aa)
|
||||
accAddrMu.RLock()
|
||||
accAddrMu.Lock()
|
||||
defer accAddrMu.Unlock()
|
||||
addr, ok := accAddrCache.Get(key)
|
||||
accAddrMu.RUnlock()
|
||||
if ok {
|
||||
return addr.(string)
|
||||
}
|
||||
return cacheBech32Addr(GetConfig().GetBech32AccountAddrPrefix(), aa, accAddrCache, key, &accAddrMu)
|
||||
return cacheBech32Addr(GetConfig().GetBech32AccountAddrPrefix(), aa, accAddrCache, key)
|
||||
}
|
||||
|
||||
// Format implements the fmt.Formatter interface.
|
||||
|
@ -418,13 +418,13 @@ func (va ValAddress) String() string {
|
|||
}
|
||||
|
||||
var key = conv.UnsafeBytesToStr(va)
|
||||
valAddrMu.RLock()
|
||||
valAddrMu.Lock()
|
||||
defer valAddrMu.Unlock()
|
||||
addr, ok := valAddrCache.Get(key)
|
||||
valAddrMu.RUnlock()
|
||||
if ok {
|
||||
return addr.(string)
|
||||
}
|
||||
return cacheBech32Addr(GetConfig().GetBech32ValidatorAddrPrefix(), va, valAddrCache, key, &valAddrMu)
|
||||
return cacheBech32Addr(GetConfig().GetBech32ValidatorAddrPrefix(), va, valAddrCache, key)
|
||||
}
|
||||
|
||||
// Format implements the fmt.Formatter interface.
|
||||
|
@ -573,13 +573,13 @@ func (ca ConsAddress) String() string {
|
|||
}
|
||||
|
||||
var key = conv.UnsafeBytesToStr(ca)
|
||||
consAddrMu.RLock()
|
||||
consAddrMu.Lock()
|
||||
defer consAddrMu.Unlock()
|
||||
addr, ok := consAddrCache.Get(key)
|
||||
consAddrMu.RUnlock()
|
||||
if ok {
|
||||
return addr.(string)
|
||||
}
|
||||
return cacheBech32Addr(GetConfig().GetBech32ConsensusAddrPrefix(), ca, consAddrCache, key, &consAddrMu)
|
||||
return cacheBech32Addr(GetConfig().GetBech32ConsensusAddrPrefix(), ca, consAddrCache, key)
|
||||
}
|
||||
|
||||
// Bech32ifyAddressBytes returns a bech32 representation of address bytes.
|
||||
|
@ -725,13 +725,12 @@ func addressBytesFromHexString(address string) ([]byte, error) {
|
|||
return hex.DecodeString(address)
|
||||
}
|
||||
|
||||
func cacheBech32Addr(prefix string, addr []byte, cache *simplelru.LRU, cacheKey string, m sync.Locker) string {
|
||||
// cacheBech32Addr is not concurrency safe. Concurrent access to cache causes race condition.
|
||||
func cacheBech32Addr(prefix string, addr []byte, cache *simplelru.LRU, cacheKey string) string {
|
||||
bech32Addr, err := bech32.ConvertAndEncode(prefix, addr)
|
||||
if err != nil {
|
||||
panic(err)
|
||||
}
|
||||
m.Lock()
|
||||
cache.Add(cacheKey, bech32Addr)
|
||||
m.Unlock()
|
||||
return bech32Addr
|
||||
}
|
||||
|
|
|
@ -0,0 +1,55 @@
|
|||
package types_test
|
||||
|
||||
import (
|
||||
"encoding/binary"
|
||||
"fmt"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/cosmos/cosmos-sdk/types"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
// generates AccAddress with `prefix` and calls String method
|
||||
func addressStringCaller(require *require.Assertions, prefix byte, max uint32, cancel chan bool, done chan<- bool) {
|
||||
bz := make([]byte, 5) // prefix + 4 bytes for uint
|
||||
bz[0] = prefix
|
||||
for i := uint32(0); ; i++ {
|
||||
if i >= max {
|
||||
i = 0
|
||||
}
|
||||
select {
|
||||
case <-cancel:
|
||||
done <- true
|
||||
return
|
||||
default:
|
||||
binary.BigEndian.PutUint32(bz[1:], i)
|
||||
str := types.AccAddress(bz).String()
|
||||
require.True(str != "")
|
||||
}
|
||||
|
||||
}
|
||||
}
|
||||
|
||||
func (s *addressTestSuite) TestAddressRace() {
|
||||
fmt.Println("starting test")
|
||||
if testing.Short() {
|
||||
s.T().Skip("AddressRace test is not short")
|
||||
}
|
||||
workers := 4
|
||||
done := make(chan bool, workers)
|
||||
cancel := make(chan bool)
|
||||
for i := byte(1); i <= 2; i++ { // workes which will loop in first 100 addresses
|
||||
go addressStringCaller(s.Require(), i, 100, cancel, done)
|
||||
}
|
||||
for i := byte(1); i <= 2; i++ { // workes which will generate 1e6 new addresses
|
||||
go addressStringCaller(s.Require(), i, 1000000, cancel, done)
|
||||
}
|
||||
<-time.After(time.Millisecond * 30)
|
||||
close(cancel)
|
||||
|
||||
// cleanup
|
||||
for i := 0; i < 4; i++ {
|
||||
<-done
|
||||
}
|
||||
}
|
Loading…
Reference in New Issue