Merge pull request #3431 from karalabe/miner-race-fixes

Miner race fixes

3 files changed, 26 insertions, 21 deletions

diff --git a/miner/miner.go b/miner/miner.go
index 87568ac18..61cd3e049 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/remote_agent.go b/miner/remote_agent.go
index 00b5f7e08..1a27a1312 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()
diff --git a/miner/worker.go b/miner/worker.go
index 5fa7c4115..fdc6b7d8e 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()