faiface / pixel

A hand-crafted 2D game library in Go
MIT License
4.46k stars 246 forks source link

Race condition in pixelgl.NewGLTriangles #68

Closed mewmew closed 7 years ago

mewmew commented 7 years ago

I've been playing around with integrating pixel into a game lately, and stumbled upon what looks like a race condition; a nil-pointer dereference in glpixel.GLTriangle.Len.

To identify the cause of the race condition, I added a few debug print statements in gltriangle.go:

// NewGLTriangles returns GLTriangles initialized with the data from the supplied Triangles.
//
// Only draw the Triangles using the provided Shader.
func NewGLTriangles(shader *glhf.Shader, t pixel.Triangles) *GLTriangles {
    var gt *GLTriangles
    fmt.Println("before")
    mainthread.Call(func() {
        fmt.Println("   in 1")
        gt = &GLTriangles{
            vs:     glhf.MakeVertexSlice(shader, 0, t.Len()),
            shader: shader,
        }
        fmt.Println("   in 2")
    })
    fmt.Printf("gt: %p\n", gt)
    gt.SetLen(t.Len())
    gt.Update(t)
    return gt
}

Right before the nil-pointer dereference, the output looks as follows:

before
   in 1
   in 2
gt: 0x47c4c280
before
   in 1
gt: 0x0
   in 2

panic: runtime error: invalid memory address or nil pointer dereference
[signal 0xc0000005 code=0x0 addr=0x0 pc=0x688de23d]

goroutine 4 [running]:
github.com/faiface/pixel/pixelgl.(*GLTriangles).Len(...)
    /home/u/goget/src/github.com/faiface/pixel/pixelgl/gltriangles.go:61
github.com/faiface/pixel/pixelgl.(*GLTriangles).SetLen(0x0, 0x6)
    /home/u/goget/src/github.com/faiface/pixel/pixelgl/gltriangles.go:69 +0x1d
github.com/faiface/pixel/pixelgl.NewGLTriangles(0x47cac0c0, 0x689cc660, 0x47caa080, 0x0)
    /home/u/goget/src/github.com/faiface/pixel/pixelgl/gltriangles.go:42 +0x146
github.com/faiface/pixel/pixelgl.(*Canvas).MakeTriangles(0x47c3e280, 0x689cc660, 0x47caa080, 0x6898c801, 0x47e14150)
    /home/u/goget/src/github.com/faiface/pixel/pixelgl/canvas.go:65 +0x38
github.com/faiface/pixel/pixelgl.(*Window).MakeTriangles(0x47cb8000, 0x689cc660, 0x47caa080, 0x47e14198, 0x0)
    /home/u/goget/src/github.com/faiface/pixel/pixelgl/window.go:368 +0x34
github.com/faiface/pixel.(*Drawer).Draw(0x47c52264, 0x689cc110, 0x47cb8000)
    /home/u/goget/src/github.com/faiface/pixel/drawer.go:74 +0x1fa
github.com/faiface/pixel.(*Sprite).DrawColorMask(0x47c52240, 0x689cc110, 0x47cb8000, 0x0, 0x3ff00000, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
    /home/u/goget/src/github.com/faiface/pixel/sprite.go:87 +0xfc
github.com/faiface/pixel.(*Sprite).Draw(0x47c52240, 0x689cc110, 0x47cb8000, 0x0, 0x3ff00000, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
    /home/u/goget/src/github.com/faiface/pixel/sprite.go:61 +0x4e

Has anyone else encountered this?

mewmew commented 7 years ago

With the following patch to mainthread/mainthread.go, I can no longer reproduce the race condition. Before I could reproduce it with high confidence running for less than 30 seconds. Now, I've had it run for 5 minutes and no race condition noticed.

diff --git a/mainthread.go b/mainthread.go
index ef29ed9..04e3347 100644
--- a/mainthread.go
+++ b/mainthread.go
@@ -60,11 +60,12 @@ func CallNonBlock(f func()) {
 // Call queues function f on the main thread and blocks until the function f finishes.
 func Call(f func()) {
        checkRun()
+       done := make(chan struct{})
        callQueue <- func() {
                f()
-               respChan <- struct{}{}
+               done <- struct{}{}
        }
-       <-respChan
+       <-done
 }

 // CallErr queues function f on the main thread and returns an error returned by f.