clean up the concurrency in Score

core/bandmap/bandmap.go

@@ -181,8 +181,8 @@ func (m *Bandmap) ContestChanged(contest core.Contest) {
 }
 
 func (m *Bandmap) ScoreUpdated(score core.Score) {
+	totalScore := score.Result()
 	m.do <- func() {
-		totalScore := score.Result()
 		m.weights.TotalPoints = float64(totalScore.Points)
 		m.weights.TotalMultis = float64(totalScore.Multis)
 		m.update()

core/core.go

@@ -483,6 +483,22 @@ func NewScore() Score {
 	}
 }
 
+func (s Score) Copy() Score {
+	result := Score{
+		ScorePerBand: make(map[Band]BandScore),
+		GraphPerBand: make(map[Band]BandGraph),
+	}
+
+	for band, bandScore := range s.ScorePerBand {
+		result.ScorePerBand[band] = bandScore
+	}
+	for band, bandGraph := range s.GraphPerBand {
+		result.GraphPerBand[band] = bandGraph.Copy()
+	}
+
+	return result
+}
+
 func (s Score) String() string {
 	buf := bytes.NewBufferString("")
 	fmt.Fprintf(buf, "Band QSOs  Dupe Pts     P/Q  Mult Q/M  Result \n")
@@ -564,6 +580,20 @@ func NewBandGraph(band Band, startTime time.Time, duration time.Duration) BandGr
 	}
 }
 
+func (g BandGraph) Copy() BandGraph {
+	result := BandGraph{
+		Band:       g.Band,
+		DataPoints: make([]BandScore, len(g.DataPoints)),
+		Max:        g.Max,
+		startTime:  g.startTime,
+		binSeconds: g.binSeconds,
+	}
+
+	copy(result.DataPoints, g.DataPoints)
+
+	return result
+}
+
 func (g BandGraph) String() string {
 	points := make([]string, len(g.DataPoints))
 	multis := make([]string, len(g.DataPoints))

core/logbook/qsolist.go

@@ -62,7 +62,8 @@ func (f ExchangeFieldsChangedListenerFunc) ExchangeFieldsChanged(myExchangeField
 
 type QSOScorer interface {
 	Clear()
-	Add(qso core.QSO) core.QSOScore
+	AddMuted(qso core.QSO) core.QSOScore
+	Unmute()
 }
 
 type QSOList struct {
@@ -139,7 +140,6 @@ func (l *QSOList) Fill(qsos []core.QSO) {
 	for _, qso := range qsos {
 		l.put(qso)
 	}
-	l.refreshScore()
 	allQSOs := l.all()
 
 	l.dataLock.Unlock()
@@ -180,6 +180,7 @@ func (l *QSOList) put(qso core.QSO) func() {
 	qsos := l.all()
 
 	return func() {
+		l.scorer.Unmute()
 		l.emitQSOsCleared()
 		for _, qso := range qsos {
 			l.emitQSOAdded(qso)
@@ -213,7 +214,7 @@ func findIndex(list []core.QSO, number core.QSONumber) (int, bool) {
 }
 
 func (l *QSOList) append(qso core.QSO) func() {
-	score := l.scorer.Add(qso)
+	score := l.scorer.AddMuted(qso)
 	qso.Points = score.Points
 	qso.Multis = score.Multis
 	qso.Duplicate = score.Duplicate
@@ -225,6 +226,7 @@ func (l *QSOList) append(qso core.QSO) func() {
 	l.list = append(l.list, qso)
 
 	return func() {
+		l.scorer.Unmute()
 		l.emitQSOAdded(qso)
 	}
 }
@@ -243,7 +245,7 @@ func (l *QSOList) refreshScore() {
 	l.dupes = make(dupeIndex)
 	l.worked = make(dupeIndex)
 	for i, qso := range l.list {
-		score := l.scorer.Add(qso)
+		score := l.scorer.AddMuted(qso)
 		qso.Points = score.Points
 		qso.Multis = score.Multis
 		qso.Duplicate = score.Duplicate

core/logbook/qsolist_test.go

@@ -306,7 +306,7 @@ func (s *testScorer) Clear() {
 	s.worked = make([]string, 1)
 }
 
-func (s *testScorer) Add(qso core.QSO) core.QSOScore {
+func (s *testScorer) AddMuted(qso core.QSO) core.QSOScore {
 	if s.scores == nil {
 		s.scores = make(map[string]core.QSOScore)
 	}
@@ -325,3 +325,5 @@ func (s *testScorer) Add(qso core.QSO) core.QSOScore {
 	s.worked = append(s.worked, callsign)
 	return result
 }
+
+func (s *testScorer) Unmute() {}

core/score/score.go

@@ -50,9 +50,10 @@ var toConvalMode = map[core.Mode]conval.Mode{
 }
 
 type Counter struct {
-	core.Score
-	counter        convalCounter
-	counterMutex   *sync.RWMutex
+	score          core.Score
+	readScore      core.Score
+	scoreLock      *sync.Mutex
+	counter        *safeConvalCounter
 	view           View
 	prefixDatabase prefixDatabase
 	invalid        bool
@@ -71,22 +72,27 @@ type Counter struct {
 
 func NewCounter(settings core.Settings, entities DXCCEntities) *Counter {
 	result := &Counter{
-		Score:          core.NewScore(),
-		counter:        new(nullCounter),
-		counterMutex:   new(sync.RWMutex),
+		score:          core.NewScore(),
+		scoreLock:      &sync.Mutex{},
+		counter:        newSafeCounter(new(nullCounter)),
 		view:           new(nullView),
 		prefixDatabase: prefixDatabase{entities},
 	}
 
+	result.copyScore()
 	result.setStation(settings.Station())
 	result.setContest(settings.Contest())
-	result.resetCounter()
+	result.resetCounter() // CONVAL WRITE LOCK
 
 	return result
 }
 
+func (c *Counter) copyScore() {
+	c.readScore = c.score.Copy() // READ
+}
+
 func (c *Counter) Result() int {
-	return c.Score.Result().Result()
+	return c.readScore.Result().Result() // READ
 }
 
 func (c *Counter) SetView(view View) {
@@ -96,15 +102,15 @@ func (c *Counter) SetView(view View) {
 	}
 	c.view = view
 	c.view.SetGoals(c.contestPointsGoal, c.contestMultisGoal)
-	c.view.ShowScore(c.Score)
+	c.view.ShowScore(c.readScore) // READ
 }
 
 func (c *Counter) StationChanged(station core.Station) {
 	oldSetup := c.contestSetup
 	c.setStation(station)
 	c.invalid = (oldSetup.MyCountry != c.contestSetup.MyCountry)
 
-	c.resetCounter()
+	c.resetCounter() // CONVAL WRITE LOCK
 }
 
 func (c *Counter) setStation(station core.Station) {
@@ -128,7 +134,7 @@ func (c *Counter) ContestChanged(contest core.Contest) {
 	c.view.SetGoals(c.contestPointsGoal, c.contestMultisGoal)
 	c.invalid = true
 
-	c.resetCounter()
+	c.resetCounter() // CONVAL WRITE LOCK
 }
 
 func (c *Counter) setContest(contest core.Contest) {
@@ -140,26 +146,14 @@ func (c *Counter) setContest(contest core.Contest) {
 	c.theirExchangeFields = toConvalExchangeFields(contest.TheirExchangeFields)
 }
 
-func (c *Counter) resetCounter() {
-	c.counterMutex.Lock()
-	defer c.counterMutex.Unlock()
-
-	if c.contestDefinition == nil {
-		c.counter = new(nullCounter)
-		return
-	}
-
-	c.counter = conval.NewCounter(*c.contestDefinition, c.contestSetup, c.prefixDatabase)
-}
-
 func (c *Counter) Valid() bool {
 	return !c.invalid && (c.contestSetup.MyCountry != "") && (c.contestSetup.MyContinent != "")
 }
 
 func (c *Counter) Show() {
 	c.view.Show()
 	c.view.SetGoals(c.contestPointsGoal, c.contestMultisGoal)
-	c.view.ShowScore(c.Score)
+	c.view.ShowScore(c.readScore) // READ
 }
 
 func (c *Counter) Hide() {
@@ -171,53 +165,57 @@ func (c *Counter) Notify(listener interface{}) {
 }
 
 func (c *Counter) Clear() {
-	c.Score = core.NewScore()
-
-	c.resetCounter()
+	c.scoreLock.Lock()
+	c.score = core.NewScore() // WRITE
+	c.copyScore()
+	c.scoreLock.Unlock()
+	c.resetCounter() // CONVAL WRITE LOCK
 
 	c.invalid = (c.contestSetup.MyCountry == "")
-	c.emitScoreUpdated(c.Score)
-}
 
-func (c *Counter) countQSO(qso core.QSO) conval.QSOScore {
-	c.counterMutex.Lock()
-	defer c.counterMutex.Unlock()
-	return c.counter.Add(c.toConvalQSO(qso))
+	c.emitScoreUpdated(c.readScore) // READ
 }
 
-func (c *Counter) Add(qso core.QSO) core.QSOScore {
-	qsoScore := c.countQSO(qso)
+func (c *Counter) AddMuted(qso core.QSO) core.QSOScore {
+	qsoScore := c.counter.Add(c.toConvalQSO(qso)) // CONVAL WRITE LOCK
 	result := core.QSOScore{
 		Points:    qsoScore.Points,
 		Multis:    qsoScore.Multis,
 		Duplicate: qsoScore.Duplicate,
 	}
 
-	bandScore := c.ScorePerBand[qso.Band]
+	c.scoreLock.Lock()
+
+	bandScore := c.score.ScorePerBand[qso.Band] // PREPARE WRITE
 	bandScore.AddQSO(result)
-	c.ScorePerBand[qso.Band] = bandScore
+	c.score.ScorePerBand[qso.Band] = bandScore // WRITE
 
 	if c.contestDefinition != nil {
-		graph, ok := c.GraphPerBand[qso.Band]
+		graph, ok := c.score.GraphPerBand[qso.Band] // PREPARE WRITE
 		if !ok {
 			graph = core.NewBandGraph(qso.Band, c.contestStartTime, c.contestDefinition.Duration)
 		}
 		graph.Add(qso.Time, result)
-		c.GraphPerBand[graph.Band] = graph
+		c.score.GraphPerBand[graph.Band] = graph // WRITE
 
-		sumGraph, ok := c.GraphPerBand[core.NoBand]
+		sumGraph, ok := c.score.GraphPerBand[core.NoBand] // PREPARE WRITE
 		if !ok {
 			sumGraph = core.NewBandGraph(core.NoBand, c.contestStartTime, c.contestDefinition.Duration)
 		}
 		sumGraph.Add(qso.Time, result)
-		c.GraphPerBand[core.NoBand] = sumGraph
+		c.score.GraphPerBand[core.NoBand] = sumGraph // WRITE
 	}
 
-	c.emitScoreUpdated(c.Score)
+	c.copyScore()
+	c.scoreLock.Unlock()
 
 	return result
 }
 
+func (c *Counter) Unmute() {
+	c.emitScoreUpdated(c.readScore)
+}
+
 func (c *Counter) emitScoreUpdated(score core.Score) {
 	c.view.ShowScore(score)
 	for _, listener := range c.listeners {
@@ -237,17 +235,11 @@ func (c *Counter) Value(callsign callsign.Callsign, entity dxcc.Prefix, band cor
 		Mode:           toConvalMode[mode],
 		TheirExchange:  c.toQSOExchange(c.theirExchangeFields, exchange),
 	}
-	qsoScore := c.probeQSO(convalQSO)
+	qsoScore := c.counter.Probe(convalQSO) // CONVAL READ LOCK
 
 	return qsoScore.Points, qsoScore.Multis, qsoScore.MultiValues
 }
 
-func (c *Counter) probeQSO(qso conval.QSO) conval.QSOScore {
-	c.counterMutex.RLock()
-	defer c.counterMutex.RUnlock()
-	return c.counter.Probe(qso)
-}
-
 func (c *Counter) toConvalQSO(qso core.QSO) conval.QSO {
 	result := conval.QSO{
 		TheirCall:     qso.Callsign,
@@ -269,6 +261,51 @@ func (c *Counter) toQSOExchange(fields []conval.ExchangeField, values []string)
 	return conval.ParseExchange(fields, values, c.prefixDatabase, c.contestDefinition)
 }
 
+func (c *Counter) resetCounter() {
+	var counter convalCounter
+	if c.contestDefinition == nil {
+		counter = new(nullCounter)
+	} else {
+		counter = conval.NewCounter(*c.contestDefinition, c.contestSetup, c.prefixDatabase)
+	}
+	c.counter.Reset(counter) // CONVAL WRITE LOCK
+}
+
+type safeConvalCounter struct {
+	counter convalCounter
+	lock    *sync.RWMutex
+}
+
+func newSafeCounter(counter convalCounter) *safeConvalCounter {
+	return &safeConvalCounter{
+		counter: counter,
+		lock:    &sync.RWMutex{},
+	}
+}
+
+func (c *safeConvalCounter) Reset(counter convalCounter) {
+	c.lock.Lock()
+	defer c.lock.Unlock()
+
+	if counter == nil {
+		c.counter = new(nullCounter)
+	} else {
+		c.counter = counter
+	}
+}
+
+func (c *safeConvalCounter) Add(qso conval.QSO) conval.QSOScore {
+	c.lock.Lock()
+	defer c.lock.Unlock()
+	return c.counter.Add(qso)
+}
+
+func (c *safeConvalCounter) Probe(qso conval.QSO) conval.QSOScore {
+	c.lock.RLock()
+	defer c.lock.RUnlock()
+	return c.counter.Probe(qso)
+}
+
 func toConvalExchangeFields(fields []core.ExchangeField) []conval.ExchangeField {
 	result := make([]conval.ExchangeField, len(fields))
 	for i, field := range fields {

core/score/score_test.go

@@ -22,8 +22,8 @@ func TestNewCounter(t *testing.T) {
 	assert.True(t, counter.Valid(), "valid")
 	assert.Equal(t, strings.ToLower(myTestEntity.entity.PrimaryPrefix), string(counter.contestSetup.MyCountry), "station entity")
 
-	counter.ScorePerBand[core.Band80m] = core.BandScore{QSOs: 5}
-	assert.Equal(t, 5, counter.ScorePerBand[core.Band80m].QSOs)
+	counter.score.ScorePerBand[core.Band80m] = core.BandScore{QSOs: 5}
+	assert.Equal(t, 5, counter.score.ScorePerBand[core.Band80m].QSOs)
 }
 
 func TestConvalCounter(t *testing.T) {
@@ -55,13 +55,13 @@ func TestConvalCounter(t *testing.T) {
 
 func TestAdd(t *testing.T) {
 	counter := NewCounter(&testSettings{stationCallsign: "DL1AAA"}, &myTestEntity)
-	counter.Add(core.QSO{Callsign: callsign.MustParse("DL0ABC"), Band: core.Band80m, DXCC: dxcc.Prefix{Prefix: "DL", PrimaryPrefix: "DL", Continent: "EU", CQZone: 14, ITUZone: 28}})
-	counter.Add(core.QSO{Callsign: callsign.MustParse("DF0ABC"), Band: core.Band80m, DXCC: dxcc.Prefix{Prefix: "DF", PrimaryPrefix: "DL", Continent: "EU", CQZone: 14, ITUZone: 28}})
+	counter.AddMuted(core.QSO{Callsign: callsign.MustParse("DL0ABC"), Band: core.Band80m, DXCC: dxcc.Prefix{Prefix: "DL", PrimaryPrefix: "DL", Continent: "EU", CQZone: 14, ITUZone: 28}})
+	counter.AddMuted(core.QSO{Callsign: callsign.MustParse("DF0ABC"), Band: core.Band80m, DXCC: dxcc.Prefix{Prefix: "DF", PrimaryPrefix: "DL", Continent: "EU", CQZone: 14, ITUZone: 28}})
 
-	assert.Equal(t, 2, counter.Score.Result().QSOs, "total QSOs")
+	assert.Equal(t, 2, counter.score.Result().QSOs, "total QSOs")
 
-	assert.Equal(t, 1, len(counter.ScorePerBand))
-	bandScore := counter.ScorePerBand[core.Band80m]
+	assert.Equal(t, 1, len(counter.score.ScorePerBand))
+	bandScore := counter.score.ScorePerBand[core.Band80m]
 	assert.Equal(t, 2, bandScore.QSOs, "band QSOs")
 }