From f86707713c42da02b70a3a389684e19e902d8759 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Szil=C3=A1gyi?= Date: Tue, 9 Jun 2015 14:56:27 +0300 Subject: [PATCH] eth: fix data race accessing peer.td --- eth/handler.go | 4 ++-- eth/peer.go | 41 ++++++++++++++++++++++++++++++----------- eth/sync.go | 2 +- 3 files changed, 33 insertions(+), 14 deletions(-) diff --git a/eth/handler.go b/eth/handler.go index 847e7a0e8..f2027c3c6 100644 --- a/eth/handler.go +++ b/eth/handler.go @@ -356,7 +356,7 @@ func (pm *ProtocolManager) importBlock(p *peer, block *types.Block, td *big.Int) p.blockHashes.Add(hash) p.SetHead(hash) if td != nil { - p.td = td + p.SetTd(td) } // Log the block's arrival _, chainHead, _ := pm.chainman.Status() @@ -369,7 +369,7 @@ func (pm *ProtocolManager) importBlock(p *peer, block *types.Block, td *big.Int) }) // If the block's already known or its difficulty is lower than ours, drop if pm.chainman.HasBlock(hash) { - p.td = pm.chainman.GetBlock(hash).Td // update the peer's TD to the real value + p.SetTd(pm.chainman.GetBlock(hash).Td) // update the peer's TD to the real value return nil } if td != nil && pm.chainman.Td().Cmp(td) > 0 && new(big.Int).Add(block.Number(), big.NewInt(7)).Cmp(pm.chainman.CurrentBlock().Number()) < 0 { diff --git a/eth/peer.go b/eth/peer.go index 5f5007891..c7045282b 100644 --- a/eth/peer.go +++ b/eth/peer.go @@ -41,10 +41,10 @@ type peer struct { protv, netid int id string - td *big.Int - head common.Hash - headLock sync.RWMutex + head common.Hash + td *big.Int + lock sync.RWMutex genesis, ourHash common.Hash ourTd *big.Int @@ -72,8 +72,8 @@ func newPeer(protv, netid int, genesis, head common.Hash, td *big.Int, p *p2p.Pe // Head retrieves a copy of the current head (most recent) hash of the peer. func (p *peer) Head() (hash common.Hash) { - p.headLock.RLock() - defer p.headLock.RUnlock() + p.lock.RLock() + defer p.lock.RUnlock() copy(hash[:], p.head[:]) return hash @@ -81,12 +81,28 @@ func (p *peer) Head() (hash common.Hash) { // SetHead updates the head (most recent) hash of the peer. func (p *peer) SetHead(hash common.Hash) { - p.headLock.Lock() - defer p.headLock.Unlock() + p.lock.Lock() + defer p.lock.Unlock() copy(p.head[:], hash[:]) } +// Td retrieves the current total difficulty of a peer. +func (p *peer) Td() *big.Int { + p.lock.RLock() + defer p.lock.RUnlock() + + return new(big.Int).Set(p.td) +} + +// SetTd updates the current total difficulty of a peer. +func (p *peer) SetTd(td *big.Int) { + p.lock.Lock() + defer p.lock.Unlock() + + p.td.Set(td) +} + // sendTransactions sends transactions to the peer and includes the hashes // in it's tx hash set for future reference. The tx hash will allow the // manager to check whether the peer has already received this particular @@ -275,11 +291,14 @@ func (ps *peerSet) BestPeer() *peer { ps.lock.RLock() defer ps.lock.RUnlock() - var best *peer + var ( + bestPeer *peer + bestTd *big.Int + ) for _, p := range ps.peers { - if best == nil || p.td.Cmp(best.td) > 0 { - best = p + if td := p.Td(); bestPeer == nil || td.Cmp(bestTd) > 0 { + bestPeer, bestTd = p, td } } - return best + return bestPeer } diff --git a/eth/sync.go b/eth/sync.go index b3184364f..3a33fe149 100644 --- a/eth/sync.go +++ b/eth/sync.go @@ -208,7 +208,7 @@ func (pm *ProtocolManager) synchronise(peer *peer) { return } // Make sure the peer's TD is higher than our own. If not drop. - if peer.td.Cmp(pm.chainman.Td()) <= 0 { + if peer.Td().Cmp(pm.chainman.Td()) <= 0 { return } // FIXME if we have the hash in our chain and the TD of the peer is