From 52820e2b43903f5460f21f46eeb9038cd0973260 Mon Sep 17 00:00:00 2001 From: Hajime Hoshi Date: Mon, 15 Jul 2024 13:40:37 +0900 Subject: [PATCH] audio: reland: bug fix: crash with uncomparable source Closes #3039 --- audio/audio.go | 14 +++++++++++++- audio/audio_test.go | 29 +++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/audio/audio.go b/audio/audio.go index ba8c5f2ba..bce5d1ab6 100644 --- a/audio/audio.go +++ b/audio/audio.go @@ -37,6 +37,7 @@ import ( "errors" "fmt" "io" + "reflect" "runtime" "sync" "time" @@ -188,11 +189,22 @@ func (c *Context) addPlayingPlayer(p *playerImpl) { defer c.m.Unlock() c.playingPlayers[p] = struct{}{} + // (reflect.Type).Comparable() is enough here, as reflect.TypeOf should always return a dynamic (non-interface) type. + // If reflect.TypeOf returned an interface type, this check would be meaningless. + // See these for more details: + // * https://pkg.go.dev/reflect#TypeOf + // * https://pkg.go.dev/reflect#Type.Comparable + // + // (*reflect.Value).Comparable() is more intuitive but this was introduced in Go 1.20. + if !reflect.TypeOf(p.sourceIdent()).Comparable() { + return + } + // Check the source duplication srcs := map[any]struct{}{} for p := range c.playingPlayers { if _, ok := srcs[p.sourceIdent()]; ok { - c.err = errors.New("audio: a same source is used by multiple Player") + c.err = errors.New("audio: the same source must not be used by multiple Player objects") return } srcs[p.sourceIdent()] = struct{}{} diff --git a/audio/audio_test.go b/audio/audio_test.go index 6aa0c95a3..7ce175fb9 100644 --- a/audio/audio_test.go +++ b/audio/audio_test.go @@ -16,6 +16,7 @@ package audio_test import ( "bytes" + "io" "os" "runtime" "testing" @@ -147,4 +148,32 @@ func TestNonSeekableSource(t *testing.T) { p.Play() p.Pause() + + if err := audio.UpdateForTesting(); err != nil { + t.Error(err) + } +} + +type uncomparableSource []int + +func (uncomparableSource) Read(buf []byte) (int, error) { + return 0, io.EOF +} + +// Issue #3039 +func TestUncomparableSource(t *testing.T) { + setup() + defer teardown() + + p, err := context.NewPlayer(uncomparableSource{}) + if err != nil { + t.Fatal(err) + } + + p.Play() + p.Pause() + + if err := audio.UpdateForTesting(); err != nil { + t.Error(err) + } }