Data race - Githubissues

davidmanzanares commented 4 years ago

Hi,

After some unexpected crashes that pointed to an issue with Go's GC (I assume something is wrong in my code or on one of my dependencies), I ran my program with -race and found the data race I left at the end.

After seeing this, I think there is a race condition between an internal beep go-routine and the one I have making calls like these:

var mixer *beep.Mixer
var buffer *beep.Buffer

func soundPlay(name string) {
    mixer.Add(buffer.Streamer(0, buffer.Len()))
}

Is this expected / do I have to call something to manage proper sync? Is this a bug?

Thank you very much :)

WARNING: DATA RACE
Read at 0x00c0004d5040 by goroutine 46:
  github.com/faiface/beep.(*bufferStreamer).Stream()
      /home/dv/go/pkg/mod/github.com/faiface/beep@v1.0.2/buffer.go:236 +0x219
  github.com/faiface/beep.(*Mixer).Stream()
      /home/dv/go/pkg/mod/github.com/faiface/beep@v1.0.2/mixer.go:42 +0x3aa
  github.com/faiface/beep/effects.(*Volume).Stream()
      /home/dv/go/pkg/mod/github.com/faiface/beep@v1.0.2/effects/volume.go:31 +0x7b
  github.com/faiface/beep.(*Mixer).Stream()
      /home/dv/go/pkg/mod/github.com/faiface/beep@v1.0.2/mixer.go:42 +0x3aa
  github.com/faiface/beep/speaker.update()
      /home/dv/go/pkg/mod/github.com/faiface/beep@v1.0.2/speaker/speaker.go:109 +0x93
  github.com/faiface/beep/speaker.Init.func1()
      /home/dv/go/pkg/mod/github.com/faiface/beep@v1.0.2/speaker/speaker.go:52 +0x31

Previous write at 0x00c0004d5040 by main goroutine:
  github.com/faiface/beep.(*Buffer).Streamer()
      /home/dv/go/pkg/mod/github.com/faiface/beep@v1.0.2/buffer.go:215 +0x1b76
  gitlab.com/david_manzanares/project3/game.soundPlay()
      /home/dv/Code/project3/game/sound.go:18 +0x1983
  gitlab.com/david_manzanares/project3/game.initEverything.func2()
      /home/dv/Code/project3/game/main.go:935 +0x1982
  gitlab.com/david_manzanares/project3/input.(*Input).dispatchHardEvent()
      /home/dv/Code/project3/input/input.go:125 +0x16f
  gitlab.com/david_manzanares/project3/input.New.func2()
      /home/dv/Code/project3/input/input.go:73 +0xa9
  github.com/go-gl/glfw/v3.3/glfw.goMouseButtonCB()
      /home/dv/go/pkg/mod/github.com/go-gl/glfw/v3.3/glfw@v0.0.0-20191125211704-12ad95a8df72/input.go:334 +0x97
  github.com/go-gl/glfw/v3.3/glfw._cgoexpwrap_6907b74fa1f9_goMouseButtonCB()
      _cgo_gotypes.go:2558 +0x50
  runtime.call32()
      /usr/local/go/src/runtime/asm_amd64.s:539 +0x3a
  github.com/go-gl/glfw/v3.3/glfw.PollEvents()
      /home/dv/go/pkg/mod/github.com/go-gl/glfw/v3.3/glfw@v0.0.0-20191125211704-12ad95a8df72/window.go:948 +0x33
  gitlab.com/david_manzanares/project3/game.(*Game).DoFrame()
      /home/dv/Code/project3/game/main.go:784 +0x2da0
  gitlab.com/david_manzanares/project3/game.(*Game).Loop()
      /home/dv/Code/project3/game/main.go:383 +0x3a
  main.main()
      /home/dv/Code/project3/main.go:7 +0x34

Goroutine 46 (running) created at:
  github.com/faiface/beep/speaker.Init()
      /home/dv/go/pkg/mod/github.com/faiface/beep@v1.0.2/speaker/speaker.go:48 +0x2ef
  gitlab.com/david_manzanares/project3/game.initSound()
      /home/dv/Code/project3/game/sound.go:32 +0x1a4
  gitlab.com/david_manzanares/project3/game.initEverything()
      /home/dv/Code/project3/game/main.go:882 +0x1bc
  gitlab.com/david_manzanares/project3/game.CreateGame()
      /home/dv/Code/project3/game/main.go:306 +0xd0
  main.main()
      /home/dv/Code/project3/main.go:6 +0x2f

Initialization code that may be useful:

    speaker.Init(format.SampleRate, format.SampleRate.N(time.Second/100))
    mixer = &beep.Mixer{}
    buffer = beep.NewBuffer(format)
    buffer.Append(streamer)
    streamer.Close()

    volume := &effects.Volume{
        Streamer: mixer,
        Base:     2,
        Volume:   -3,
        Silent:   false,
    }
    speaker.Play(volume)

faiface commented 4 years ago

Hi, thanks for reporting this! Are you making the calls concurrently? If you are calling Mixed.Add multiple times concurrently, that will cause a race condition. In general, no Beep object (except for the speaker) is concurrency-safe and if you're using it concurrently, you'll have to guard it with mutexes or something.

faiface commented 4 years ago

Oh, I just realized what probably is the issue. You're modifying a streamer that's currently playing, right? That will cause a race condition because the speaker will pull data from the streamer at the same time as you are modifying it.

To prevent this, use speaker.Lock(), that will prevent the speaker from pulling data, modify your streamer and then call speaker.Unlock() to let the speaker pull data again.

faiface / beep

Data race #83