From e59378b45dfee044e6fa8c6634d5f467785929ff Mon Sep 17 00:00:00 2001 From: Alexander Bezobchuk Date: Thu, 25 Jun 2020 10:03:59 -0400 Subject: [PATCH] p2p: Remove data race bug in netaddr stringer (#5048) ## Description Remove concurrent write access bug by removing the memoized string representation of a `NetAddress`. Closes: #5047 --- p2p/netaddress.go | 24 ++++++++++-------------- p2p/netaddress_test.go | 23 +++++++++++++++++++++++ 2 files changed, 33 insertions(+), 14 deletions(-) diff --git a/p2p/netaddress.go b/p2p/netaddress.go index 5a17788ab..77209217b 100644 --- a/p2p/netaddress.go +++ b/p2p/netaddress.go @@ -17,18 +17,15 @@ import ( tmp2p "github.com/tendermint/tendermint/proto/tendermint/p2p" ) +// EmptyNetAddress defines the string representation of an empty NetAddress +const EmptyNetAddress = "" + // NetAddress defines information about a peer on the network // including its ID, IP address, and port. type NetAddress struct { ID ID `json:"id"` IP net.IP `json:"ip"` Port uint16 `json:"port"` - - // TODO: - // Name string `json:"name"` // optional DNS name - - // memoize .String() - str string } // IDAddressString returns id@hostPort. It strips the leading @@ -213,16 +210,15 @@ func (na *NetAddress) Same(other interface{}) bool { // String representation: @: func (na *NetAddress) String() string { if na == nil { - return "" + return EmptyNetAddress } - if na.str == "" { - addrStr := na.DialString() - if na.ID != "" { - addrStr = IDAddressString(na.ID, addrStr) - } - na.str = addrStr + + addrStr := na.DialString() + if na.ID != "" { + addrStr = IDAddressString(na.ID, addrStr) } - return na.str + + return addrStr } func (na *NetAddress) DialString() string { diff --git a/p2p/netaddress_test.go b/p2p/netaddress_test.go index 4a9ef333d..65f9fb834 100644 --- a/p2p/netaddress_test.go +++ b/p2p/netaddress_test.go @@ -2,12 +2,35 @@ package p2p import ( "net" + "sync" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) +func TestNetAddress_String(t *testing.T) { + tcpAddr, err := net.ResolveTCPAddr("tcp", "127.0.0.1:8080") + require.Nil(t, err) + + netAddr := NewNetAddress("deadbeefdeadbeefdeadbeefdeadbeefdeadbeef", tcpAddr) + + var wg sync.WaitGroup + + for i := 0; i < 10; i++ { + wg.Add(1) + go func() { + defer wg.Done() + _ = netAddr.String() + }() + } + + wg.Wait() + + s := netAddr.String() + require.Equal(t, "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef@127.0.0.1:8080", s) +} + func TestNewNetAddress(t *testing.T) { tcpAddr, err := net.ResolveTCPAddr("tcp", "127.0.0.1:8080") require.Nil(t, err)