Fix race condition in munin output.

The munin server used the same structure in two goroutines at once causing
possible data corruption. A copy of the structure is now used by the second
goroutine instead.
This commit is contained in:
Hendrik van Wyk
2020-09-26 13:35:11 +02:00
parent 157736a99d
commit 49be089a23
3 changed files with 65 additions and 43 deletions

View File

@@ -39,9 +39,6 @@ gofmt:
gofmt -l -s -w . gofmt -l -s -w .
test: test:
go test -v ./...
test-race:
go test -v -race ./... go test -v -race ./...
docker: docker:

View File

@@ -44,18 +44,18 @@ var log = logrus.WithField("ctx", "inverter-gui-munin")
type Munin struct { type Munin struct {
mk2driver.Mk2 mk2driver.Mk2
muninResponse chan *muninData muninResponse chan muninData
} }
type muninData struct { type muninData struct {
status *mk2driver.Mk2Info status mk2driver.Mk2Info
timesUpdated int timesUpdated int
} }
func NewMunin(mk2 mk2driver.Mk2) *Munin { func NewMunin(mk2 mk2driver.Mk2) *Munin {
m := &Munin{ m := &Munin{
Mk2: mk2, Mk2: mk2,
muninResponse: make(chan *muninData), muninResponse: make(chan muninData),
} }
go m.run() go m.run()
@@ -71,10 +71,10 @@ func (m *Munin) ServeMuninHTTP(rw http.ResponseWriter, r *http.Request) {
_, _ = rw.Write([]byte("No data to return.\n")) _, _ = rw.Write([]byte("No data to return.\n"))
return return
} }
calcMuninAverages(muninDat) calcMuninAverages(&muninDat)
status := muninDat.status status := muninDat.status
tmpInput := buildTemplateInput(status) tmpInput := buildTemplateInput(&status)
outputBuf := &bytes.Buffer{} outputBuf := &bytes.Buffer{}
fmt.Fprintf(outputBuf, "multigraph in_batvolt\n") fmt.Fprintf(outputBuf, "multigraph in_batvolt\n")
fmt.Fprintf(outputBuf, "volt.value %s\n", tmpInput.BatVoltage) fmt.Fprintf(outputBuf, "volt.value %s\n", tmpInput.BatVoltage)
@@ -113,65 +113,61 @@ func (m *Munin) ServeMuninConfigHTTP(rw http.ResponseWriter, r *http.Request) {
func (m *Munin) run() { func (m *Munin) run() {
muninValues := &muninData{ muninValues := &muninData{
status: &mk2driver.Mk2Info{}, status: mk2driver.Mk2Info{},
} }
for { for {
select { select {
case e := <-m.C(): case e := <-m.C():
if e.Valid { if e.Valid {
calcMuninValues(muninValues, e) calcMuninValues(muninValues, e)
} }
case m.muninResponse <- muninValues: case m.muninResponse <- *muninValues:
zeroMuninValues(muninValues) zeroMuninValues(muninValues)
} }
} }
} }
//Munin only samples once every 5 minutes so averages have to be calculated for some values. //Munin only samples once every 5 minutes so averages have to be calculated for some values.
func calcMuninValues(muninDat *muninData, newStatus *mk2driver.Mk2Info) { func calcMuninValues(m *muninData, newStatus *mk2driver.Mk2Info) {
muninDat.timesUpdated++ m.timesUpdated++
muninVal := muninDat.status m.status.OutCurrent += newStatus.OutCurrent
muninVal.OutCurrent += newStatus.OutCurrent m.status.InCurrent += newStatus.InCurrent
muninVal.InCurrent += newStatus.InCurrent m.status.BatCurrent += newStatus.BatCurrent
muninVal.BatCurrent += newStatus.BatCurrent
muninVal.OutVoltage += newStatus.OutVoltage m.status.OutVoltage += newStatus.OutVoltage
muninVal.InVoltage += newStatus.InVoltage m.status.InVoltage += newStatus.InVoltage
muninVal.BatVoltage += newStatus.BatVoltage m.status.BatVoltage += newStatus.BatVoltage
muninVal.InFrequency = newStatus.InFrequency m.status.InFrequency = newStatus.InFrequency
muninVal.OutFrequency = newStatus.OutFrequency m.status.OutFrequency = newStatus.OutFrequency
muninVal.ChargeState = newStatus.ChargeState m.status.ChargeState = newStatus.ChargeState
} }
func calcMuninAverages(muninDat *muninData) { func calcMuninAverages(m *muninData) {
muninVal := muninDat.status m.status.OutCurrent /= float64(m.timesUpdated)
muninVal.OutCurrent /= float64(muninDat.timesUpdated) m.status.InCurrent /= float64(m.timesUpdated)
muninVal.InCurrent /= float64(muninDat.timesUpdated) m.status.BatCurrent /= float64(m.timesUpdated)
muninVal.BatCurrent /= float64(muninDat.timesUpdated)
muninVal.OutVoltage /= float64(muninDat.timesUpdated) m.status.OutVoltage /= float64(m.timesUpdated)
muninVal.InVoltage /= float64(muninDat.timesUpdated) m.status.InVoltage /= float64(m.timesUpdated)
muninVal.BatVoltage /= float64(muninDat.timesUpdated) m.status.BatVoltage /= float64(m.timesUpdated)
} }
func zeroMuninValues(muninDat *muninData) { func zeroMuninValues(m *muninData) {
muninDat.timesUpdated = 0 m.timesUpdated = 0
muninVal := muninDat.status m.status.OutCurrent = 0
muninVal.OutCurrent = 0 m.status.InCurrent = 0
muninVal.InCurrent = 0 m.status.BatCurrent = 0
muninVal.BatCurrent = 0
muninVal.OutVoltage = 0 m.status.OutVoltage = 0
muninVal.InVoltage = 0 m.status.InVoltage = 0
muninVal.BatVoltage = 0 m.status.BatVoltage = 0
muninVal.InFrequency = 0 m.status.InFrequency = 0
muninVal.OutFrequency = 0 m.status.OutFrequency = 0
muninVal.ChargeState = 0 m.status.ChargeState = 0
} }
type templateInput struct { type templateInput struct {

View File

@@ -0,0 +1,29 @@
package munin
import (
"io/ioutil"
"net/http"
"net/http/httptest"
"testing"
"github.com/diebietse/invertergui/mk2driver"
)
func TestServer(t *testing.T) {
mockMk2 := mk2driver.NewMk2Mock()
muninServer := NewMunin(mockMk2)
ts := httptest.NewServer(http.HandlerFunc(muninServer.ServeMuninHTTP))
defer ts.Close()
res, err := http.Get(ts.URL)
if err != nil {
log.Fatal(err)
}
_, err = ioutil.ReadAll(res.Body)
res.Body.Close()
if err != nil {
log.Fatal(err)
}
}