From 0a7b2ab52c3cf80187eaf839d70db875f7403419 Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Fri, 20 Oct 2017 21:56:10 +0400 Subject: [PATCH 1/4] fix invalid memory address or nil pointer dereference error (Refs #762) https://github.com/tendermint/tendermint/issues/762#issuecomment-338276055 ``` E[10-19|04:52:38.969] Stopping peer for error module=p2p peer="Peer{MConn{178.62.46.14:46656} B14916FAF38A out}" err="Error: runtime error: invalid memory address or nil pointer dereference\nStack: goroutine 529485 [running]:\nruntime/debug.Stack(0xc4355cfb38, 0xb463e0, 0x11b1c30)\n\t/usr/local/go/src/runtime/debug/stack.go:24 +0xa7\ngithub.com/cosmos/gaia/vendor/github.com/tendermint/tendermint/p2p.(*MConnection)._recover(0xc439a28870)\n\t/home/ubuntu/go/src/github.com/cosmos/gaia/vendor/github.com/tendermint/tendermint/p2p/connection.go:206 +0x6e\npanic(0xb463e0, 0x11b1c30)\n\t/usr/local/go/src/runtime/panic.go:491 +0x283\ngithub.com/cosmos/gaia/vendor/github.com/tendermint/tendermint/blockchain.(*bpPeer).decrPending(0x0, 0x381)\n\t/home/ubuntu/go/src/github.com/cosmos/gaia/vendor/github.com/tendermint/tendermint/blockchain/pool.go:376 +0x22\ngithub.com/cosmos/gaia/vendor/github.com/tendermint/tendermint/blockchain.(*BlockPool).AddBlock(0xc4200e4000, 0xc4266d1f00, 0x40, 0xc432ac9640, 0x381)\n\t/home/ubuntu/go/src/github.com/cosmos/gaia/vendor/github.com/tendermint/tendermint/blockchain/pool.go:215 +0x139\ngithub.com/cosmos/gaia/vendor/github.com/tendermint/tendermint/blockchain.(*BlockchainReactor).Receive(0xc42050a780, 0xc420257740, 0x1171be0, 0xc42ff302d0, 0xc4384b2000, 0x381, 0x1000)\n\t/home/ubuntu/go/src/github.com/cosmos/gaia/vendor/github.com/tendermint/tendermint/blockchain/reactor.go:160 +0x712\ngithub.com/cosmos/gaia/vendor/github.com/tendermint/tendermint/p2p.createMConnection.func1(0x11e5040, 0xc4384b2000, 0x381, 0x1000)\n\t/home/ubuntu/go/src/github.com/cosmos/gaia/vendor/github.com/tendermint/tendermint/p2p/peer.go:334 +0xbd\ngithub.com/cosmos/gaia/vendor/github.com/tendermint/tendermint/p2p.(*MConnection).recvRoutine(0xc439a28870)\n\t/home/ubuntu/go/src/github.com/cosmos/gaia/vendor/github.com/tendermint/tendermint/p2p/connection.go:475 +0x4a3\ncreated by github.com/cosmos/gaia/vendor/github.com/tendermint/tendermint/p2p.(*MConnection).OnStart\n\t/home/ubuntu/go/src/github.com/cosmos/gaia/vendor/github.com/tendermint/tendermint/p2p/connection.go:170 +0x187\n" ``` --- blockchain/pool.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/blockchain/pool.go b/blockchain/pool.go index bd52e280..ca15c991 100644 --- a/blockchain/pool.go +++ b/blockchain/pool.go @@ -212,7 +212,9 @@ func (pool *BlockPool) AddBlock(peerID string, block *types.Block, blockSize int if requester.setBlock(block, peerID) { pool.numPending-- peer := pool.peers[peerID] - peer.decrPending(blockSize) + if peer != nil { + peer.decrPending(blockSize) + } } else { // Bad peer? } From d64a48e0ee3419c48c1a2c91c20c34ccc48c53d2 Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Fri, 20 Oct 2017 23:56:21 +0400 Subject: [PATCH 2/4] set logger on blockchain pool --- blockchain/pool.go | 4 +--- blockchain/reactor.go | 11 +++++++++-- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/blockchain/pool.go b/blockchain/pool.go index ca15c991..aac4c77c 100644 --- a/blockchain/pool.go +++ b/blockchain/pool.go @@ -69,9 +69,7 @@ func (pool *BlockPool) OnStart() error { return nil } -func (pool *BlockPool) OnStop() { - pool.BaseService.OnStop() -} +func (pool *BlockPool) OnStop() {} // Run spawns requesters as needed. func (pool *BlockPool) makeRequestersRoutine() { diff --git a/blockchain/reactor.go b/blockchain/reactor.go index 9cc01fba..6ff01003 100644 --- a/blockchain/reactor.go +++ b/blockchain/reactor.go @@ -12,6 +12,7 @@ import ( sm "github.com/tendermint/tendermint/state" "github.com/tendermint/tendermint/types" cmn "github.com/tendermint/tmlibs/common" + "github.com/tendermint/tmlibs/log" ) const ( @@ -79,7 +80,13 @@ func NewBlockchainReactor(state *sm.State, proxyAppConn proxy.AppConnConsensus, return bcR } -// OnStart implements BaseService +// SetLogger implements cmn.Service by setting the logger on reactor and pool. +func (bcR *BlockchainReactor) SetLogger(l log.Logger) { + bcR.BaseService.Logger = l + bcR.pool.Logger = l +} + +// OnStart implements cmn.Service. func (bcR *BlockchainReactor) OnStart() error { bcR.BaseReactor.OnStart() if bcR.fastSync { @@ -92,7 +99,7 @@ func (bcR *BlockchainReactor) OnStart() error { return nil } -// OnStop implements BaseService +// OnStop implements cmn.Service. func (bcR *BlockchainReactor) OnStop() { bcR.BaseReactor.OnStop() bcR.pool.Stop() From 0bbf38141a6498f661d6c956c9f45066c312d90e Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Fri, 20 Oct 2017 21:19:25 -0400 Subject: [PATCH 3/4] blockchain/pool: some comments and small changes --- blockchain/pool.go | 23 ++++++++++++++++++++--- blockchain/reactor.go | 4 ++-- p2p/pex_reactor.go | 1 + 3 files changed, 23 insertions(+), 5 deletions(-) diff --git a/blockchain/pool.go b/blockchain/pool.go index aac4c77c..348ba09b 100644 --- a/blockchain/pool.go +++ b/blockchain/pool.go @@ -11,11 +11,25 @@ import ( "github.com/tendermint/tmlibs/log" ) +/* + +eg, L = latency = 0.1s + P = num peers = 10 + FN = num full nodes + BS = 1kB block size + CB = 1 Mbit/s = 128 kB/s + CB/P = 12.8 kB + B/S = CB/P/BS = 12.8 blocks/s + + 12.8 * 0.1 = 1.28 blocks on conn + +*/ + const ( requestIntervalMS = 250 maxTotalRequesters = 300 maxPendingRequests = maxTotalRequesters - maxPendingRequestsPerPeer = 75 + maxPendingRequestsPerPeer = 10 minRecvRate = 10240 // 10Kb/s ) @@ -186,15 +200,16 @@ func (pool *BlockPool) PopRequest() { // Remove the peer and redo request from others. func (pool *BlockPool) RedoRequest(height int) { pool.mtx.Lock() + defer pool.mtx.Unlock() + request := pool.requesters[height] - pool.mtx.Unlock() if request.block == nil { cmn.PanicSanity("Expected block to be non-nil") } // RemovePeer will redo all requesters associated with this peer. // TODO: record this malfeasance - pool.RemovePeer(request.peerID) + pool.removePeer(request.peerID) } // TODO: ensure that blocks come in order for each peer. @@ -204,6 +219,8 @@ func (pool *BlockPool) AddBlock(peerID string, block *types.Block, blockSize int requester := pool.requesters[block.Height] if requester == nil { + // a block we didn't expect. + // TODO:if height is too far ahead, punish peer return } diff --git a/blockchain/reactor.go b/blockchain/reactor.go index 6ff01003..b46ad40f 100644 --- a/blockchain/reactor.go +++ b/blockchain/reactor.go @@ -221,9 +221,9 @@ FOR_LOOP: // ask for status updates go bcR.BroadcastStatusRequest() case <-switchToConsensusTicker.C: - height, numPending, _ := bcR.pool.GetStatus() + height, numPending, lenRequesters := bcR.pool.GetStatus() outbound, inbound, _ := bcR.Switch.NumPeers() - bcR.Logger.Info("Consensus ticker", "numPending", numPending, "total", len(bcR.pool.requesters), + bcR.Logger.Info("Consensus ticker", "numPending", numPending, "total", lenRequesters, "outbound", outbound, "inbound", inbound) if bcR.pool.IsCaughtUp() { bcR.Logger.Info("Time to switch to consensus reactor!", "height", height) diff --git a/p2p/pex_reactor.go b/p2p/pex_reactor.go index 69ab55cc..54c2d06b 100644 --- a/p2p/pex_reactor.go +++ b/p2p/pex_reactor.go @@ -139,6 +139,7 @@ func (r *PEXReactor) Receive(chID byte, src Peer, msgBytes []byte) { switch msg := msg.(type) { case *pexRequestMessage: // src requested some peers. + // NOTE: we might send an empty selection r.SendAddrs(src, r.book.GetSelection()) case *pexAddrsMessage: // We received some peer addresses from src. From e06bbaf3036d529b84b737dfd3513999085aa493 Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Tue, 24 Oct 2017 15:32:01 +0400 Subject: [PATCH 4/4] refactor TestNoBlockMessageResponse to eliminate a race condition --- blockchain/reactor_test.go | 27 ++++++++++----------------- 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/blockchain/reactor_test.go b/blockchain/reactor_test.go index 633cae16..584aadf3 100644 --- a/blockchain/reactor_test.go +++ b/blockchain/reactor_test.go @@ -1,12 +1,11 @@ package blockchain import ( - "bytes" "testing" wire "github.com/tendermint/go-wire" cmn "github.com/tendermint/tmlibs/common" - "github.com/tendermint/tmlibs/db" + dbm "github.com/tendermint/tmlibs/db" "github.com/tendermint/tmlibs/log" cfg "github.com/tendermint/tendermint/config" @@ -15,28 +14,24 @@ import ( "github.com/tendermint/tendermint/types" ) -func newBlockchainReactor(logger log.Logger, maxBlockHeight int) *BlockchainReactor { - config := cfg.ResetTestRoot("node_node_test") +func newBlockchainReactor(maxBlockHeight int) *BlockchainReactor { + logger := log.TestingLogger() + config := cfg.ResetTestRoot("blockchain_reactor_test") - blockStoreDB := db.NewDB("blockstore", config.DBBackend, config.DBDir()) - blockStore := NewBlockStore(blockStoreDB) - - stateLogger := logger.With("module", "state") + blockStore := NewBlockStore(dbm.NewMemDB()) // Get State - stateDB := db.NewDB("state", config.DBBackend, config.DBDir()) - state, _ := sm.GetState(stateDB, config.GenesisFile()) - - state.SetLogger(stateLogger) + state, _ := sm.GetState(dbm.NewMemDB(), config.GenesisFile()) + state.SetLogger(logger.With("module", "state")) state.Save() // Make the blockchainReactor itself fastSync := true bcReactor := NewBlockchainReactor(state.Copy(), nil, blockStore, fastSync) + bcReactor.SetLogger(logger.With("module", "blockchain")) // Next: we need to set a switch in order for peers to be added in bcReactor.Switch = p2p.NewSwitch(cfg.DefaultP2PConfig()) - bcReactor.SetLogger(logger.With("module", "blockchain")) // Lastly: let's add some blocks in for blockHeight := 1; blockHeight <= maxBlockHeight; blockHeight++ { @@ -50,12 +45,10 @@ func newBlockchainReactor(logger log.Logger, maxBlockHeight int) *BlockchainReac } func TestNoBlockMessageResponse(t *testing.T) { - logBuf := new(bytes.Buffer) - logger := log.NewTMLogger(logBuf) maxBlockHeight := 20 - bcr := newBlockchainReactor(logger, maxBlockHeight) - go bcr.OnStart() + bcr := newBlockchainReactor(maxBlockHeight) + bcr.Start() defer bcr.Stop() // Add some peers in