audio: bug fix: deadlock between a player and a context

Closes #2737

audio/audio.go

@@ -198,22 +198,37 @@ func (c *Context) removePlayer(p *playerImpl) {
 }
 
 func (c *Context) gcPlayers() error {
+	// A Context must not call playerImpl's functions with a lock, or this causes a deadlock (#2737).
+	// Copy the playerImpls and iterate them without a lock.
+	var players []*playerImpl
 	c.m.Lock()
-	defer c.m.Unlock()
+	players = make([]*playerImpl, 0, len(c.players))
+	for p := range c.players {
+		players = append(players, p)
+	}
+	c.m.Unlock()
+
+	var playersToRemove []*playerImpl
 
 	// Now reader players cannot call removePlayers from themselves in the current implementation.
 	// Underlying playering can be the pause state after fishing its playing,
 	// but there is no way to notify this to players so far.
 	// Instead, let's check the states proactively every frame.
-	for p := range c.players {
+	for _, p := range players {
 		if err := p.Err(); err != nil {
 			return err
 		}
 		if !p.IsPlaying() {
-			delete(c.players, p)
+			playersToRemove = append(playersToRemove, p)
 		}
 	}
 
+	c.m.Lock()
+	for _, p := range playersToRemove {
+		delete(c.players, p)
+	}
+	c.m.Unlock()
+
 	return nil
 }
 

internal/processtest/testdata/issue2737.go

@@ -0,0 +1,130 @@
+// Copyright 2023 The Ebitengine Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+//go:build ignore
+
+package main
+
+import (
+	"errors"
+	"io"
+	"sync"
+	"time"
+
+	"github.com/hajimehoshi/ebiten/v2"
+	"github.com/hajimehoshi/ebiten/v2/audio"
+)
+
+type emptyStream struct {
+	length int64
+	n      int64
+}
+
+func (e *emptyStream) Seek(offset int64, whence int) (int64, error) {
+	switch whence {
+	case io.SeekStart:
+		e.n = offset
+	case io.SeekCurrent:
+		e.n += offset
+	case io.SeekEnd:
+		e.n = e.length + offset
+	}
+	if e.n > e.length || e.n < 0 {
+		return 0, errors.New("out of range")
+	}
+	return e.n, nil
+}
+
+func (e *emptyStream) Read(buf []byte) (int, error) {
+	n := int64(len(buf))
+	if e.n+n >= e.length {
+		n := e.length - e.n
+		e.n = e.length
+		return int(n), io.EOF
+	}
+	e.n += n
+	return int(n), nil
+}
+
+type Game struct {
+	playerCount         int
+	finishedPlayerCount int
+	tickCount           int
+
+	m sync.Mutex
+}
+
+func (g *Game) countUpFinishedPlayer() {
+	g.m.Lock()
+	defer g.m.Unlock()
+	g.finishedPlayerCount++
+}
+
+func (g *Game) Update() error {
+	g.tickCount++
+	if g.tickCount > 600 {
+		panic("time out")
+	}
+
+	g.m.Lock()
+	c := g.finishedPlayerCount
+	g.m.Unlock()
+	if g.playerCount == c {
+		return ebiten.Termination
+	}
+	return nil
+}
+
+func (g *Game) Draw(screen *ebiten.Image) {
+}
+
+func (g *Game) Layout(width, height int) (int, int) {
+	return width, height
+}
+
+func main() {
+	game := &Game{
+		playerCount: 1000,
+	}
+
+	ctx := audio.NewContext(48000)
+	var players []*audio.Player
+	for i := 0; i < game.playerCount; i++ {
+		p, err := ctx.NewPlayer(&emptyStream{length: 48000 * 2 * 2})
+		if err != nil {
+			panic(err)
+		}
+		players = append(players, p)
+
+		// Play players in different goroutines from the game's goroutine.
+		go func() {
+			p.Play()
+			for i := 0; i < 3; i++ {
+				for {
+					if !p.IsPlaying() {
+						p.Rewind()
+						p.Play()
+						break
+					}
+					time.Sleep(100 * time.Millisecond)
+				}
+			}
+			game.countUpFinishedPlayer()
+		}()
+	}
+
+	if err := ebiten.RunGame(game); err != nil {
+		panic(err)
+	}
+}