ebiten: running a pulled iterator can crash the game

hajimehoshi commented 1 month ago

Ebitengine Version

52820e2b43903f5460f21f46eeb9038cd0973260

Operating System

[ ] Windows
[ ] macOS
[ ] Linux
[ ] FreeBSD
[ ] OpenBSD
[ ] Android
[ ] iOS
[ ] Nintendo Switch
[ ] PlayStation 5
[ ] Xbox
[ ] Web Browsers

Go Version (`go version`)

go version go1.23rc1 darwin/arm64

What steps will reproduce the problem?

Run this program with Go 1.23 (rc1):

package main

import (
    "iter"

    "github.com/hajimehoshi/ebiten/v2"
)

type Game struct {
    next func() (int, bool)
}

func (g *Game) Update() error {
    g.next()
    return nil
}

func (g *Game) Draw(screen *ebiten.Image) {
}

func (g *Game) Layout(outsideWidth, outsideHeight int) (screenWidth, screenHeight int) {
    return outsideWidth, outsideHeight
}

func main() {
    seq := func(yield func(int) bool) {
        for {
            if !yield(0) {
                break
            }
        }
    }
    next, _ := iter.Pull(seq)
    g := &Game{
        next: next,
    }
    if err := ebiten.RunGame(g); err != nil {
        panic(err)
    }
}

What is the expected result?

No crash

What happens instead?

Crash:

coro: got thread 0x14000100008, want 0x100cce260
coro: got lock internal 0, want 0
coro: got lock external 0, want 1
fatal error: coro: OS thread locking must match locking at coroutine creation

runtime stack:
runtime.throw({0x100a106ee?, 0x14000103dc0?})
        /Users/hajimehoshi/sdk/go1.23rc1/src/runtime/panic.go:1067 +0x38 fp=0x17167ae80 sp=0x17167ae50 pc=0x100812b38
runtime.coroswitch_m(0x14000103dc0?)
        /Users/hajimehoshi/sdk/go1.23rc1/src/runtime/coro.go:125 +0x46c fp=0x17167af00 sp=0x17167ae80 pc=0x1007ae0ec
runtime.mcall()
        /Users/hajimehoshi/sdk/go1.23rc1/src/runtime/asm_arm64.s:193 +0x54 fp=0x17167af10 sp=0x17167af00 pc=0x100818204
...

Anything else you feel useful to add?

Reported by @eihigh

If needed, let's backport the fix to 2.7

hajimehoshi commented 1 month ago

The problem is that Ebitengine calls LockOSThread in an internal init function, so the goroutine for the main function already locks an OS thread.

Actually, this works without crashing:

package main

import (
    "iter"

    "github.com/hajimehoshi/ebiten/v2"
)

type Game struct {
    next func() (int, bool)
}

func (g *Game) Update() error {
    g.next()
    return nil
}

func (g *Game) Draw(screen *ebiten.Image) {
}

func (g *Game) Layout(outsideWidth, outsideHeight int) (screenWidth, screenHeight int) {
    return outsideWidth, outsideHeight
}

func main() {
    var next func() (int, bool)
    ch := make(chan struct{})
    go func() {
        seq := func(yield func(int) bool) {
            for {
                if !yield(0) {
                    break
                }
            }
        }
        next, _ = iter.Pull(seq)
        close(ch)
    }()
    <-ch
    g := &Game{
        next: next,
    }
    if err := ebiten.RunGame(g); err != nil {
        panic(err)
    }
}

hajimehoshi commented 1 month ago

We might not need LockOSThread for main. See https://github.com/golang/go/issues/64777#issuecomment-2163641860

hajimehoshi commented 1 month ago

Sorry, 3d385ef0aa9ec5dabb41076c79e1290c15da32a0 doesn't fix the issue.

hajimehoshi commented 1 month ago

I think this is not a fixable issue unless mainthread package is introduced... (golang/go#64777). Especially, some APIs for macOS must be called from the main thread, and there is no way to do it without LockOSThread in an init function.

hajimehoshi / ebiten