From 44147d057dd91d8b35dd6f4ed025bdb4baf225eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Szil=C3=A1gyi?= Date: Tue, 9 Jun 2015 14:27:44 +0300 Subject: [PATCH] eth: fix data race accessing peer.recentHash --- eth/handler.go | 6 +++--- eth/peer.go | 31 +++++++++++++++++++++++++------ eth/sync.go | 7 ++++--- 3 files changed, 32 insertions(+), 12 deletions(-) diff --git a/eth/handler.go b/eth/handler.go index 64f89b273..847e7a0e8 100644 --- a/eth/handler.go +++ b/eth/handler.go @@ -157,7 +157,7 @@ func (pm *ProtocolManager) handle(p *peer) error { } defer pm.removePeer(p.id) - if err := pm.downloader.RegisterPeer(p.id, p.recentHash, p.requestHashes, p.requestBlocks); err != nil { + if err := pm.downloader.RegisterPeer(p.id, p.Head(), p.requestHashes, p.requestBlocks); err != nil { return err } // propagate existing transactions. new transactions appearing @@ -303,7 +303,7 @@ func (self *ProtocolManager) handleMsg(p *peer) error { // Mark the hashes as present at the remote node for _, hash := range hashes { p.blockHashes.Add(hash) - p.recentHash = hash + p.SetHead(hash) } // Schedule all the unknown hashes for retrieval unknown := make([]common.Hash, 0, len(hashes)) @@ -354,7 +354,7 @@ func (pm *ProtocolManager) importBlock(p *peer, block *types.Block, td *big.Int) // Mark the block as present at the remote node (don't duplicate already held data) p.blockHashes.Add(hash) - p.recentHash = hash + p.SetHead(hash) if td != nil { p.td = td } diff --git a/eth/peer.go b/eth/peer.go index cf2c58ec9..5f5007891 100644 --- a/eth/peer.go +++ b/eth/peer.go @@ -40,9 +40,11 @@ type peer struct { protv, netid int - recentHash common.Hash - id string - td *big.Int + id string + td *big.Int + + head common.Hash + headLock sync.RWMutex genesis, ourHash common.Hash ourTd *big.Int @@ -51,14 +53,14 @@ type peer struct { blockHashes *set.Set } -func newPeer(protv, netid int, genesis, recentHash common.Hash, td *big.Int, p *p2p.Peer, rw p2p.MsgReadWriter) *peer { +func newPeer(protv, netid int, genesis, head common.Hash, td *big.Int, p *p2p.Peer, rw p2p.MsgReadWriter) *peer { id := p.ID() return &peer{ Peer: p, rw: rw, genesis: genesis, - ourHash: recentHash, + ourHash: head, ourTd: td, protv: protv, netid: netid, @@ -68,6 +70,23 @@ func newPeer(protv, netid int, genesis, recentHash common.Hash, td *big.Int, p * } } +// 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() + + copy(hash[:], p.head[:]) + return 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() + + copy(p.head[:], hash[:]) +} + // 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 @@ -160,7 +179,7 @@ func (p *peer) handleStatus() error { // Set the total difficulty of the peer p.td = status.TD // set the best hash of the peer - p.recentHash = status.CurrentBlock + p.head = status.CurrentBlock return <-errc } diff --git a/eth/sync.go b/eth/sync.go index dd7414da8..b3184364f 100644 --- a/eth/sync.go +++ b/eth/sync.go @@ -214,14 +214,15 @@ func (pm *ProtocolManager) synchronise(peer *peer) { // FIXME if we have the hash in our chain and the TD of the peer is // much higher than ours, something is wrong with us or the peer. // Check if the hash is on our own chain - if pm.chainman.HasBlock(peer.recentHash) { + head := peer.Head() + if pm.chainman.HasBlock(head) { glog.V(logger.Debug).Infoln("Synchronisation canceled: head already known") return } // Get the hashes from the peer (synchronously) - glog.V(logger.Detail).Infof("Attempting synchronisation: %v, 0x%x", peer.id, peer.recentHash) + glog.V(logger.Detail).Infof("Attempting synchronisation: %v, 0x%x", peer.id, head) - err := pm.downloader.Synchronise(peer.id, peer.recentHash) + err := pm.downloader.Synchronise(peer.id, head) switch err { case nil: glog.V(logger.Detail).Infof("Synchronisation completed")