Browse Source

Fix race when mutating MConnConfig

Instead of mutating the passed in MConnConfig part of P2PConfig we just
use the default and override the values, the same as before as it was
always the default version. This is yet another good reason to not embed
information and access to config structs in our components and will go
away with the ongoing refactoring in #1325.
pull/1702/head
Alexander Simmerl 7 years ago
parent
commit
c661a3ec21
No known key found for this signature in database GPG Key ID: 4694E95C9CC61BDA
4 changed files with 22 additions and 16 deletions
  1. +2
    -6
      config/config.go
  2. +2
    -1
      p2p/peer.go
  3. +8
    -4
      p2p/peer_test.go
  4. +10
    -5
      p2p/switch.go

+ 2
- 6
config/config.go View File

@ -5,8 +5,6 @@ import (
"os" "os"
"path/filepath" "path/filepath"
"time" "time"
tmconn "github.com/tendermint/tendermint/p2p/conn"
) )
const ( const (
@ -304,9 +302,8 @@ type P2PConfig struct {
AllowDuplicateIP bool `mapstructure:"allow_duplicate_ip"` AllowDuplicateIP bool `mapstructure:"allow_duplicate_ip"`
// Peer connection configuration. // Peer connection configuration.
HandshakeTimeout time.Duration `mapstructure:"handshake_timeout"`
DialTimeout time.Duration `mapstructure:"dial_timeout"`
MConfig tmconn.MConnConfig `mapstructure:"connection"`
HandshakeTimeout time.Duration `mapstructure:"handshake_timeout"`
DialTimeout time.Duration `mapstructure:"dial_timeout"`
// Testing params. // Testing params.
// Force dial to fail // Force dial to fail
@ -332,7 +329,6 @@ func DefaultP2PConfig() *P2PConfig {
AllowDuplicateIP: true, // so non-breaking yet AllowDuplicateIP: true, // so non-breaking yet
HandshakeTimeout: 20 * time.Second, HandshakeTimeout: 20 * time.Second,
DialTimeout: 3 * time.Second, DialTimeout: 3 * time.Second,
MConfig: tmconn.DefaultMConnConfig(),
TestDialFail: false, TestDialFail: false,
TestFuzz: false, TestFuzz: false,
TestFuzzConfig: DefaultFuzzConnConfig(), TestFuzzConfig: DefaultFuzzConnConfig(),


+ 2
- 1
p2p/peer.go View File

@ -102,6 +102,7 @@ type peer struct {
func newPeer( func newPeer(
pc peerConn, pc peerConn,
mConfig tmconn.MConnConfig,
nodeInfo NodeInfo, nodeInfo NodeInfo,
reactorsByCh map[byte]Reactor, reactorsByCh map[byte]Reactor,
chDescs []*tmconn.ChannelDescriptor, chDescs []*tmconn.ChannelDescriptor,
@ -120,7 +121,7 @@ func newPeer(
reactorsByCh, reactorsByCh,
chDescs, chDescs,
onPeerError, onPeerError,
pc.config.MConfig,
mConfig,
) )
p.BaseService = *cmn.NewBaseService(nil, "Peer", p) p.BaseService = *cmn.NewBaseService(nil, "Peer", p)


+ 8
- 4
p2p/peer_test.go View File

@ -27,7 +27,7 @@ func TestPeerBasic(t *testing.T) {
rp.Start() rp.Start()
defer rp.Stop() defer rp.Stop()
p, err := createOutboundPeerAndPerformHandshake(rp.Addr(), cfg)
p, err := createOutboundPeerAndPerformHandshake(rp.Addr(), cfg, tmconn.DefaultMConnConfig())
require.Nil(err) require.Nil(err)
err = p.Start() err = p.Start()
@ -53,7 +53,7 @@ func TestPeerSend(t *testing.T) {
rp.Start() rp.Start()
defer rp.Stop() defer rp.Stop()
p, err := createOutboundPeerAndPerformHandshake(rp.Addr(), config)
p, err := createOutboundPeerAndPerformHandshake(rp.Addr(), config, tmconn.DefaultMConnConfig())
require.Nil(err) require.Nil(err)
err = p.Start() err = p.Start()
@ -65,7 +65,11 @@ func TestPeerSend(t *testing.T) {
assert.True(p.Send(testCh, []byte("Asylum"))) assert.True(p.Send(testCh, []byte("Asylum")))
} }
func createOutboundPeerAndPerformHandshake(addr *NetAddress, config *config.P2PConfig) (*peer, error) {
func createOutboundPeerAndPerformHandshake(
addr *NetAddress,
config *config.P2PConfig,
mConfig tmconn.MConnConfig,
) (*peer, error) {
chDescs := []*tmconn.ChannelDescriptor{ chDescs := []*tmconn.ChannelDescriptor{
{ID: testCh, Priority: 1}, {ID: testCh, Priority: 1},
} }
@ -86,7 +90,7 @@ func createOutboundPeerAndPerformHandshake(addr *NetAddress, config *config.P2PC
return nil, err return nil, err
} }
p := newPeer(pc, nodeInfo, reactorsByCh, chDescs, func(p Peer, r interface{}) {})
p := newPeer(pc, mConfig, nodeInfo, reactorsByCh, chDescs, func(p Peer, r interface{}) {})
p.SetLogger(log.TestingLogger().With("peer", addr)) p.SetLogger(log.TestingLogger().With("peer", addr))
return p, nil return p, nil
} }


+ 10
- 5
p2p/switch.go View File

@ -70,6 +70,8 @@ type Switch struct {
filterConnByAddr func(net.Addr) error filterConnByAddr func(net.Addr) error
filterConnByID func(ID) error filterConnByID func(ID) error
mConfig conn.MConnConfig
rng *cmn.Rand // seed for randomizing dial times and orders rng *cmn.Rand // seed for randomizing dial times and orders
} }
@ -88,10 +90,13 @@ func NewSwitch(cfg *config.P2PConfig) *Switch {
// Ensure we have a completely undeterministic PRNG. // Ensure we have a completely undeterministic PRNG.
sw.rng = cmn.NewRand() sw.rng = cmn.NewRand()
sw.config.MConfig.FlushThrottle = time.Duration(cfg.FlushThrottleTimeout) * time.Millisecond
sw.config.MConfig.SendRate = cfg.SendRate
sw.config.MConfig.RecvRate = cfg.RecvRate
sw.config.MConfig.MaxPacketMsgPayloadSize = cfg.MaxPacketMsgPayloadSize
mConfig := conn.DefaultMConnConfig()
mConfig.FlushThrottle = time.Duration(cfg.FlushThrottleTimeout) * time.Millisecond
mConfig.SendRate = cfg.SendRate
mConfig.RecvRate = cfg.RecvRate
mConfig.MaxPacketMsgPayloadSize = cfg.MaxPacketMsgPayloadSize
sw.mConfig = mConfig
sw.BaseService = *cmn.NewBaseService(nil, "P2P Switch", sw) sw.BaseService = *cmn.NewBaseService(nil, "P2P Switch", sw)
return sw return sw
@ -600,7 +605,7 @@ func (sw *Switch) addPeer(pc peerConn) error {
return err return err
} }
peer := newPeer(pc, peerNodeInfo, sw.reactorsByCh, sw.chDescs, sw.StopPeerForError)
peer := newPeer(pc, sw.mConfig, peerNodeInfo, sw.reactorsByCh, sw.chDescs, sw.StopPeerForError)
peer.SetLogger(sw.Logger.With("peer", addr)) peer.SetLogger(sw.Logger.With("peer", addr))
peer.Logger.Info("Successful handshake with peer", "peerNodeInfo", peerNodeInfo) peer.Logger.Info("Successful handshake with peer", "peerNodeInfo", peerNodeInfo)


Loading…
Cancel
Save