gizak / termui

Golang terminal dashboard
MIT License
13.14k stars 787 forks source link

Data races in dashboard.go example #122

Closed alexozer closed 5 years ago

alexozer commented 7 years ago

It appears the Go race detector finds data races in the dashboard.go example:

$ go run -race dashboard.go
==================
WARNING: DATA RACE
Write at 0x00c4200d4388 by goroutine 19:
  main.main.func3()
      /tmp/dashboard.go:125 +0x6a0
  main.main.func5()
      /tmp/dashboard.go:133 +0x9a
  github.com/gizak/termui.(*EvtStream).Loop.func1()
      /home/alex/code/go/src/github.com/gizak/termui/events.go:251 +0x160

Previous write at 0x00c4200d4388 by goroutine 18:
  main.main.func3()
      /tmp/dashboard.go:125 +0x6a0
  main.main.func5()
      /tmp/dashboard.go:133 +0x9a
  github.com/gizak/termui.(*EvtStream).Loop.func1()
      /home/alex/code/go/src/github.com/gizak/termui/events.go:251 +0x160

Goroutine 19 (running) created at:
  github.com/gizak/termui.(*EvtStream).Loop()
      /home/alex/code/go/src/github.com/gizak/termui/events.go:253 +0x192
  github.com/gizak/termui.Loop()
      /home/alex/code/go/src/github.com/gizak/termui/events.go:278 +0x4a
  main.main()
      /tmp/dashboard.go:135 +0x1065

Goroutine 18 (finished) created at:
  github.com/gizak/termui.(*EvtStream).Loop()
      /home/alex/code/go/src/github.com/gizak/termui/events.go:253 +0x192
  github.com/gizak/termui.Loop()
      /home/alex/code/go/src/github.com/gizak/termui/events.go:278 +0x4a
  main.main()
      /tmp/dashboard.go:135 +0x1065
==================

It seems as though this may be a natural consequence of making termui.Render() async as this means writing widget properties and reading widget properties during rendering cannot be done sequentially:

draw := func(t int) {
    g.Percent = t % 101
    list.Items = strs[t%9:]
    sp.Lines[0].Data = spdata[:30+t%50]
    sp.Lines[1].Data = spdata[:35+t%50]
    lc.Data = sinps[t/2%220:]
    lc1.Data = sinps[2*t%220:]
    bc.Data = bcdata[t/2%10:]

    // Is Render() guaranteed to read widgets when we're not writing them,
    // or does it only rely in timing?
    ui.Render(p, list, g, sp, lc, bc, lc1, p1)
}
// ...
ui.Handle("/timer/1s", func(e ui.Event) {
    t := e.Data.(ui.EvtTimer)
    draw(int(t.Count))
})

Is this considered the standard way of using termui? Or maybe, does termui expect the user to pass a fresh set of widgets to every call to Render()?

cjbassi commented 5 years ago

Just tried go run -race _examples/dashboard1.go and it seems to be working for me. I just rewrote a bunch of things, including removing the async nature of Render so that probably fixed it. Thanks for pointing this out btw!