From b750cab56a724b2ff0bff3fab0849cb776a4f392 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Szil=C3=A1gyi?= Date: Tue, 13 Dec 2016 13:42:33 +0200 Subject: [PATCH 1/2] miner: fix a race between remote agent start/loop --- miner/remote_agent.go | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/miner/remote_agent.go b/miner/remote_agent.go index 00b5f7e089..1a27a13120 100644 --- a/miner/remote_agent.go +++ b/miner/remote_agent.go @@ -37,7 +37,7 @@ type hashrate struct { type RemoteAgent struct { mu sync.Mutex - quit chan struct{} + quitCh chan struct{} workCh chan *Work returnCh chan<- *Result @@ -76,18 +76,16 @@ func (a *RemoteAgent) Start() { if !atomic.CompareAndSwapInt32(&a.running, 0, 1) { return } - - a.quit = make(chan struct{}) + a.quitCh = make(chan struct{}) a.workCh = make(chan *Work, 1) - go a.maintainLoop() + go a.loop(a.workCh, a.quitCh) } func (a *RemoteAgent) Stop() { if !atomic.CompareAndSwapInt32(&a.running, 1, 0) { return } - - close(a.quit) + close(a.quitCh) close(a.workCh) } @@ -148,15 +146,20 @@ func (a *RemoteAgent) SubmitWork(nonce uint64, mixDigest, hash common.Hash) bool return false } -func (a *RemoteAgent) maintainLoop() { +// loop monitors mining events on the work and quit channels, updating the internal +// state of the rmeote miner until a termination is requested. +// +// Note, the reason the work and quit channels are passed as parameters is because +// RemoteAgent.Start() constantly recreates these channels, so the loop code cannot +// assume data stability in these member fields. +func (a *RemoteAgent) loop(workCh chan *Work, quitCh chan struct{}) { ticker := time.Tick(5 * time.Second) -out: for { select { - case <-a.quit: - break out - case work := <-a.workCh: + case <-quitCh: + return + case work := <-workCh: a.mu.Lock() a.currentWork = work a.mu.Unlock() From dadd68935935388b158a510c95b8644be44073ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Szil=C3=A1gyi?= Date: Tue, 13 Dec 2016 14:03:18 +0200 Subject: [PATCH 2/2] miner: fix data race on setting etherbase/extradata --- miner/miner.go | 16 ++++++---------- miner/worker.go | 6 ++++++ 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/miner/miner.go b/miner/miner.go index 87568ac18a..61cd3e0499 100644 --- a/miner/miner.go +++ b/miner/miner.go @@ -119,15 +119,14 @@ func (m *Miner) SetGasPrice(price *big.Int) { func (self *Miner) Start(coinbase common.Address, threads int) { atomic.StoreInt32(&self.shouldStart, 1) - self.threads = threads - self.worker.coinbase = coinbase + self.worker.setEtherbase(coinbase) self.coinbase = coinbase + self.threads = threads if atomic.LoadInt32(&self.canStart) == 0 { glog.V(logger.Info).Infoln("Can not start mining operation due to network sync (starts when finished)") return } - atomic.StoreInt32(&self.mining, 1) for i := 0; i < threads; i++ { @@ -135,9 +134,7 @@ func (self *Miner) Start(coinbase common.Address, threads int) { } glog.V(logger.Info).Infof("Starting mining operation (CPU=%d TOT=%d)\n", threads, len(self.worker.agents)) - self.worker.start() - self.worker.commitNewWork() } @@ -177,8 +174,7 @@ func (self *Miner) SetExtra(extra []byte) error { if uint64(len(extra)) > params.MaximumExtraDataSize.Uint64() { return fmt.Errorf("Extra exceeds max length. %d > %v", len(extra), params.MaximumExtraDataSize) } - - self.worker.extra = extra + self.worker.setExtra(extra) return nil } @@ -188,9 +184,9 @@ func (self *Miner) Pending() (*types.Block, *state.StateDB) { } // PendingBlock returns the currently pending block. -// -// Note, to access both the pending block and the pending state -// simultaneously, please use Pending(), as the pending state can +// +// Note, to access both the pending block and the pending state +// simultaneously, please use Pending(), as the pending state can // change between multiple method calls func (self *Miner) PendingBlock() *types.Block { return self.worker.pendingBlock() diff --git a/miner/worker.go b/miner/worker.go index 5fa7c4115a..fdc6b7d8e6 100644 --- a/miner/worker.go +++ b/miner/worker.go @@ -161,6 +161,12 @@ func (self *worker) setEtherbase(addr common.Address) { self.coinbase = addr } +func (self *worker) setExtra(extra []byte) { + self.mu.Lock() + defer self.mu.Unlock() + self.extra = extra +} + func (self *worker) pending() (*types.Block, *state.StateDB) { self.currentMu.Lock() defer self.currentMu.Unlock()