beep icon indicating copy to clipboard operation
beep copied to clipboard

Data race

Open davidmanzanares opened this issue 4 years ago • 2 comments

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/[email protected]/buffer.go:236 +0x219
  github.com/faiface/beep.(*Mixer).Stream()
      /home/dv/go/pkg/mod/github.com/faiface/[email protected]/mixer.go:42 +0x3aa
  github.com/faiface/beep/effects.(*Volume).Stream()
      /home/dv/go/pkg/mod/github.com/faiface/[email protected]/effects/volume.go:31 +0x7b
  github.com/faiface/beep.(*Mixer).Stream()
      /home/dv/go/pkg/mod/github.com/faiface/[email protected]/mixer.go:42 +0x3aa
  github.com/faiface/beep/speaker.update()
      /home/dv/go/pkg/mod/github.com/faiface/[email protected]/speaker/speaker.go:109 +0x93
  github.com/faiface/beep/speaker.Init.func1()
      /home/dv/go/pkg/mod/github.com/faiface/[email protected]/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/[email protected]/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/[email protected]/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/[email protected]/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/[email protected]/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)

davidmanzanares avatar Apr 14 '20 23:04 davidmanzanares

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 avatar Apr 14 '20 23:04 faiface

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 avatar Apr 14 '20 23:04 faiface