hajimehoshi / ebiten

Ebitengine - A dead simple 2D game engine for Go
https://ebitengine.org
Apache License 2.0
11.02k stars 660 forks source link

internal/graphics: `Vertices` and `QuadVertices` are not concurrent-safe #2473

Closed hajimehoshi closed 1 year ago

hajimehoshi commented 1 year ago

Ebitengine Version

d898bb1ce4f2cb6a819e00c0b5bd30ef1e638775

Operating System

Go Version (go version)

go version go1.19.2 darwin/amd64

What steps will reproduce the problem?

-race sometimes detects the race condition in this function.

What is the expected result?

No race condition.

What happens instead?

A race condition is detected at https://github.com/hajimehoshi/ebiten/blob/2.4/internal/graphics/vertex.go#L153

WARNING: DATA RACE
Write at 0x00c00035c100 by goroutine 24:
  github.com/hajimehoshi/ebiten/v2/internal/graphics.QuadVertices()
      /home/sion/pkg/mod/github.com/hajimehoshi/ebiten/v2@v2.4.12/internal/graphics/vertex.go:153 +0x135
  github.com/hajimehoshi/ebiten/v2.(*Image).DrawImage()
      /home/sion/pkg/mod/github.com/hajimehoshi/ebiten/v2@v2.4.12/image.go:310 +0x837
  github.com/hajimehoshi/ebiten/v2/ebitenutil.drawDebugText()
      /home/sion/pkg/mod/github.com/hajimehoshi/ebiten/v2@v2.4.12/ebitenutil/debugprint.go:81 +0x4cf
  github.com/hajimehoshi/ebiten/v2/ebitenutil.DebugPrintAt()
      /home/sion/pkg/mod/github.com/hajimehoshi/ebiten/v2@v2.4.12/ebitenutil/debugprint.go:52 +0x44
  github.com/hajimehoshi/ebiten/v2/ebitenutil.DebugPrint()
      /home/sion/pkg/mod/github.com/hajimehoshi/ebiten/v2@v2.4.12/ebitenutil/debugprint.go:45 +0x26
  main.(*LoadingScreen).Draw()
      /home/sion/src/github.com/sinisterstuf/escort-mission/loadingscreen.go:22 +0x25
  main.(*Game).Draw()
      /home/sion/src/github.com/sinisterstuf/escort-mission/main.go:142 +0xd2
  github.com/hajimehoshi/ebiten/v2.(*imageDumperGame).Draw()
      /home/sion/pkg/mod/github.com/hajimehoshi/ebiten/v2@v2.4.12/run.go:162 +0x70
  github.com/hajimehoshi/ebiten/v2.(*gameForUI).Draw()
      /home/sion/pkg/mod/github.com/hajimehoshi/ebiten/v2@v2.4.12/gameforui.go:65 +0x81
  github.com/hajimehoshi/ebiten/v2/internal/ui.(*context).drawGame()
      /home/sion/pkg/mod/github.com/hajimehoshi/ebiten/v2@v2.4.12/internal/ui/context.go:159 +0x306
  github.com/hajimehoshi/ebiten/v2/internal/ui.(*context).updateFrameImpl()
      /home/sion/pkg/mod/github.com/hajimehoshi/ebiten/v2@v2.4.12/internal/ui/context.go:139 +0x453
  github.com/hajimehoshi/ebiten/v2/internal/ui.(*context).updateFrame()
      /home/sion/pkg/mod/github.com/hajimehoshi/ebiten/v2@v2.4.12/internal/ui/context.go:63 +0x7c
  github.com/hajimehoshi/ebiten/v2/internal/ui.(*userInterfaceImpl).loop()
      /home/sion/pkg/mod/github.com/hajimehoshi/ebiten/v2@v2.4.12/internal/ui/ui_glfw.go:1126 +0x5ae
  github.com/hajimehoshi/ebiten/v2/internal/ui.(*userInterfaceImpl).Run.func1()
      /home/sion/pkg/mod/github.com/hajimehoshi/ebiten/v2@v2.4.12/internal/ui/run_notsinglethread.go:46 +0x295

Previous write at 0x00c00035c100 by goroutine 23:
  github.com/hajimehoshi/ebiten/v2/internal/graphics.QuadVertices()
      /home/sion/pkg/mod/github.com/hajimehoshi/ebiten/v2@v2.4.12/internal/graphics/vertex.go:153 +0x135
  github.com/hajimehoshi/ebiten/v2.(*Image).DrawImage()
      /home/sion/pkg/mod/github.com/hajimehoshi/ebiten/v2@v2.4.12/image.go:310 +0x837
  main.(*TileRenderer).Render()
      /home/sion/src/github.com/sinisterstuf/escort-mission/renderer.go:162 +0x4ed
  main.NewGameScreen()
      /home/sion/src/github.com/sinisterstuf/escort-mission/gamescreen.go:108 +0xa6c
  main.main.func1()
      /home/sion/src/github.com/sinisterstuf/escort-mission/main.go:51 +0x39

Goroutine 24 (running) created at:
  github.com/hajimehoshi/ebiten/v2/internal/ui.(*userInterfaceImpl).Run()
      /home/sion/pkg/mod/github.com/hajimehoshi/ebiten/v2@v2.4.12/internal/ui/run_notsinglethread.go:33 +0x3c6
  github.com/hajimehoshi/ebiten/v2.RunGame()
      /home/sion/pkg/mod/github.com/hajimehoshi/ebiten/v2@v2.4.12/run.go:204 +0x20d
  main.main()
      /home/sion/src/github.com/sinisterstuf/escort-mission/main.go:53 +0x615

Goroutine 23 (running) created at:
  main.main()
      /home/sion/src/github.com/sinisterstuf/escort-mission/main.go:51 +0x604

Anything else you feel useful to add?

@sinisterstuf reported this.

hajimehoshi commented 1 year ago

Hmm? They use a mutex in slice, so this seems concurrent safe.

hajimehoshi commented 1 year ago

I think I found the cause, but the fix would not be obvious.

sionleroux commented 1 year ago

I'm not sure the output from the race detector and the stack trace after panicking are 100% the same issue but they seem related. Full logs of both can be found here: https://gist.github.com/sinisterstuf/644ef4d47aba4f5737e540001305c5cb

hajimehoshi commented 1 year ago

@sinisterstuf Is there a minimum program to reproduce this issue? Thanks!

sionleroux commented 1 year ago

Not yet but I can try to write one. Currently the crashes are coming from running this game as-is, however it is rare, and unpredictable when the crash will occur because of the nature of the data race. Maybe on Friday I can try to write a small program that does concurrent draw calls, similarly to this, but with much higher load, so as to increase the chances of a panic.

hajimehoshi commented 1 year ago

As 1) this is pretty rare, and 2) we need nonsignificant amount of work to fix this, I'll postpone to fix this to v2.5.

hajimehoshi commented 1 year ago

Please try 61f1d8b69f144c84ba8fe8914cd51309ca7bf822. Thanks

sionleroux commented 1 year ago

Yes, I can confirm that using this version the go tool does not detect any data races and the game does not crash ☺️🎉