Protect ProposerPolicy Registry using Mutex (Consensys#1242)

* Protect access to validator set in ProposerPolicy using mutex

* Making Registry private

* Moving eventLoop() to after calling Seal() as earlier stop channel might have triggered before calling Seal()

consensus/istanbul/backend/engine_test.go

@@ -169,14 +169,14 @@ func TestSealStopChannel(t *testing.T) {
 		stop <- struct{}{}
 		eventSub.Unsubscribe()
 	}
-	go eventLoop()
 	resultCh := make(chan *types.Block, 10)
 	go func() {
 		err := engine.Seal(chain, block, resultCh, stop)
 		if err != nil {
 			t.Errorf("error mismatch: have %v, want nil", err)
 		}
 	}()
+	go eventLoop()
 
 	finalBlock := <-resultCh
 	if finalBlock != nil {

consensus/istanbul/backend/snapshot.go

@@ -211,7 +211,7 @@ func (s *Snapshot) UnmarshalJSON(b []byte) error {
 	s.Tally = j.Tally
 
 	// Setting the By function to ValidatorSortByStringFunc should be fine, as the validator do not change only the order changes
-	pp := &istanbul.ProposerPolicy{Id: j.Policy, By: istanbul.ValidatorSortByString()}
+	pp := istanbul.NewProposerPolicyByIdAndSortFunc(j.Policy, istanbul.ValidatorSortByString())
 	s.ValSet = validator.NewSet(j.Validators, pp)
 	return nil
 }

consensus/istanbul/config.go

@@ -18,6 +18,7 @@ package istanbul
 
 import (
 	"math/big"
+	"sync"
 
 	"github.com/naoina/toml"
 )
@@ -31,9 +32,10 @@ const (
 
 // ProposerPolicy represents the Validator Proposer Policy
 type ProposerPolicy struct {
-	Id       ProposerPolicyId    // Could be RoundRobin or Sticky
-	Registry []ValidatorSet      // Holds the ValidatorSet for a given block height
-	By       ValidatorSortByFunc // func that defines how the ValidatorSet should be sorted
+	Id         ProposerPolicyId    // Could be RoundRobin or Sticky
+	By         ValidatorSortByFunc // func that defines how the ValidatorSet should be sorted
+	registry   []ValidatorSet      // Holds the ValidatorSet for a given block height
+	registryMU *sync.Mutex         // Mutex to lock access to changes to Registry
 }
 
 // NewRoundRobinProposerPolicy returns a RoundRobin ProposerPolicy with ValidatorSortByString as default sort function
@@ -47,7 +49,11 @@ func NewStickyProposerPolicy() *ProposerPolicy {
 }
 
 func NewProposerPolicy(id ProposerPolicyId) *ProposerPolicy {
-	return &ProposerPolicy{Id: id, By: ValidatorSortByString()}
+	return NewProposerPolicyByIdAndSortFunc(id, ValidatorSortByString())
+}
+
+func NewProposerPolicyByIdAndSortFunc(id ProposerPolicyId, by ValidatorSortByFunc) *ProposerPolicy {
+	return &ProposerPolicy{Id: id, By: by, registryMU: new(sync.Mutex)}
 }
 
 type proposerPolicyToml struct {
@@ -74,23 +80,29 @@ func (p *ProposerPolicy) UnmarshalTOML(input []byte) error {
 func (p *ProposerPolicy) Use(v ValidatorSortByFunc) {
 	p.By = v
 
-	for _, validatorSet := range p.Registry {
+	for _, validatorSet := range p.registry {
 		validatorSet.SortValidators()
 	}
 }
 
 // RegisterValidatorSet stores the given ValidatorSet in the policy registry
 func (p *ProposerPolicy) RegisterValidatorSet(valSet ValidatorSet) {
-	if len(p.Registry) == 0 {
-		p.Registry = []ValidatorSet{valSet}
+	p.registryMU.Lock()
+	defer p.registryMU.Unlock()
+
+	if len(p.registry) == 0 {
+		p.registry = []ValidatorSet{valSet}
 	} else {
-		p.Registry = append(p.Registry, valSet)
+		p.registry = append(p.registry, valSet)
 	}
 }
 
 // ClearRegistry removes any ValidatorSet from the ProposerPolicy registry
 func (p *ProposerPolicy) ClearRegistry() {
-	p.Registry = nil
+	p.registryMU.Lock()
+	defer p.registryMU.Unlock()
+
+	p.registry = nil
 }
 
 type Config struct {