From 853d5d3fd0a3cb7e9e6cf05da147379cfb132804 Mon Sep 17 00:00:00 2001 From: Evgeny Danienko <6655321@bk.ru> Date: Tue, 24 Oct 2017 14:39:36 +0300 Subject: [PATCH 1/3] les: wg data race fixed --- les/fetcher.go | 11 ++++++++--- les/handler.go | 12 +++++++++++- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/les/fetcher.go b/les/fetcher.go index 4fc142f0f4c7..e0d1465b0e73 100644 --- a/les/fetcher.go +++ b/les/fetcher.go @@ -20,6 +20,7 @@ package les import ( "math/big" "sync" + "sync/atomic" "time" "github.com/ethereum/go-ethereum/common" @@ -123,11 +124,15 @@ func newLightFetcher(pm *ProtocolManager) *lightFetcher { // syncLoop is the main event loop of the light fetcher func (f *lightFetcher) syncLoop() { - f.pm.wg.Add(1) - defer f.pm.wg.Done() - + once := true requesting := false for { + if once && atomic.LoadInt32(f.pm.isClosed) != closed { + once = false + f.pm.wg.Add(1) + defer f.pm.wg.Done() + } + select { case <-f.pm.quitSync: return diff --git a/les/handler.go b/les/handler.go index df7eb6af51b3..285d2aeecfdd 100644 --- a/les/handler.go +++ b/les/handler.go @@ -24,6 +24,7 @@ import ( "math/big" "net" "sync" + "sync/atomic" "time" "github.com/ethereum/go-ethereum/common" @@ -115,6 +116,7 @@ type ProtocolManager struct { // channels for fetcher, syncer, txsyncLoop newPeerCh chan *peer + isClosed *int32 quitSync chan struct{} noMorePeers chan struct{} @@ -123,10 +125,16 @@ type ProtocolManager struct { wg *sync.WaitGroup } +const ( + started int32 = iota + closed +) + // NewProtocolManager returns a new ethereum sub protocol manager. The Ethereum sub protocol manages peers capable // with the ethereum network. func NewProtocolManager(chainConfig *params.ChainConfig, lightSync bool, networkId uint64, mux *event.TypeMux, engine consensus.Engine, peers *peerSet, blockchain BlockChain, txpool txPool, chainDb ethdb.Database, odr *LesOdr, txrelay *LesTxRelay, quitSync chan struct{}, wg *sync.WaitGroup) (*ProtocolManager, error) { // Create the protocol manager with the base fields + isClosed := started manager := &ProtocolManager{ lightSync: lightSync, eventMux: mux, @@ -139,6 +147,7 @@ func NewProtocolManager(chainConfig *params.ChainConfig, lightSync bool, network txrelay: txrelay, peers: peers, newPeerCh: make(chan *peer), + isClosed: &isClosed, quitSync: quitSync, wg: wg, noMorePeers: make(chan struct{}), @@ -242,7 +251,8 @@ func (pm *ProtocolManager) Stop() { // will exit when they try to register. pm.peers.Close() - // Wait for any process action + // Wait for any process action. Wait should be executed after all pm.wg.Add() + atomic.StoreInt32(pm.isClosed, closed) pm.wg.Wait() log.Info("Light Ethereum protocol stopped") From a231fdbf790bab7eaaf060accb53a3ea259aa6c7 Mon Sep 17 00:00:00 2001 From: Evgeny Danienko <6655321@bk.ru> Date: Tue, 7 Nov 2017 14:51:38 +0300 Subject: [PATCH 2/3] les: protocol manager wait group filled in main goroutine --- les/fetcher.go | 11 +++-------- les/handler.go | 9 --------- 2 files changed, 3 insertions(+), 17 deletions(-) diff --git a/les/fetcher.go b/les/fetcher.go index e0d1465b0e73..3fc4df30b992 100644 --- a/les/fetcher.go +++ b/les/fetcher.go @@ -20,7 +20,6 @@ package les import ( "math/big" "sync" - "sync/atomic" "time" "github.com/ethereum/go-ethereum/common" @@ -118,21 +117,17 @@ func newLightFetcher(pm *ProtocolManager) *lightFetcher { maxConfirmedTd: big.NewInt(0), } pm.peers.notify(f) + + f.pm.wg.Add(1) go f.syncLoop() return f } // syncLoop is the main event loop of the light fetcher func (f *lightFetcher) syncLoop() { - once := true requesting := false + defer f.pm.wg.Done() for { - if once && atomic.LoadInt32(f.pm.isClosed) != closed { - once = false - f.pm.wg.Add(1) - defer f.pm.wg.Done() - } - select { case <-f.pm.quitSync: return diff --git a/les/handler.go b/les/handler.go index 285d2aeecfdd..caade0bef012 100644 --- a/les/handler.go +++ b/les/handler.go @@ -24,7 +24,6 @@ import ( "math/big" "net" "sync" - "sync/atomic" "time" "github.com/ethereum/go-ethereum/common" @@ -125,16 +124,10 @@ type ProtocolManager struct { wg *sync.WaitGroup } -const ( - started int32 = iota - closed -) - // NewProtocolManager returns a new ethereum sub protocol manager. The Ethereum sub protocol manages peers capable // with the ethereum network. func NewProtocolManager(chainConfig *params.ChainConfig, lightSync bool, networkId uint64, mux *event.TypeMux, engine consensus.Engine, peers *peerSet, blockchain BlockChain, txpool txPool, chainDb ethdb.Database, odr *LesOdr, txrelay *LesTxRelay, quitSync chan struct{}, wg *sync.WaitGroup) (*ProtocolManager, error) { // Create the protocol manager with the base fields - isClosed := started manager := &ProtocolManager{ lightSync: lightSync, eventMux: mux, @@ -147,7 +140,6 @@ func NewProtocolManager(chainConfig *params.ChainConfig, lightSync bool, network txrelay: txrelay, peers: peers, newPeerCh: make(chan *peer), - isClosed: &isClosed, quitSync: quitSync, wg: wg, noMorePeers: make(chan struct{}), @@ -252,7 +244,6 @@ func (pm *ProtocolManager) Stop() { pm.peers.Close() // Wait for any process action. Wait should be executed after all pm.wg.Add() - atomic.StoreInt32(pm.isClosed, closed) pm.wg.Wait() log.Info("Light Ethereum protocol stopped") From b04eb83c2f6012c56abd91c50a7b720903947c74 Mon Sep 17 00:00:00 2001 From: Evgeny Danienko <6655321@bk.ru> Date: Wed, 8 Nov 2017 13:55:03 +0300 Subject: [PATCH 3/3] les: remove unused code and comment --- les/handler.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/les/handler.go b/les/handler.go index caade0bef012..df7eb6af51b3 100644 --- a/les/handler.go +++ b/les/handler.go @@ -115,7 +115,6 @@ type ProtocolManager struct { // channels for fetcher, syncer, txsyncLoop newPeerCh chan *peer - isClosed *int32 quitSync chan struct{} noMorePeers chan struct{} @@ -243,7 +242,7 @@ func (pm *ProtocolManager) Stop() { // will exit when they try to register. pm.peers.Close() - // Wait for any process action. Wait should be executed after all pm.wg.Add() + // Wait for any process action pm.wg.Wait() log.Info("Light Ethereum protocol stopped")