From 4300b92d4b721365951f12a21b085b7ff23298b3 Mon Sep 17 00:00:00 2001 From: DualFroz Date: Sun, 4 Oct 2026 12:56:43 +0200 Subject: [PATCH] audio: pair locks with defer and move the disposal of a context to audio.go --- audio/audio.go | 13 +++++++++++++ audio/audio_test.go | 7 ++----- audio/export_test.go | 39 ++++++++++++++++++++------------------- 3 files changed, 35 insertions(+), 24 deletions(-) diff --git a/audio/audio.go b/audio/audio.go index ded88b2..a7a9f4c 100644 --- a/audio/audio.go +++ b/audio/audio.go @@ -233,6 +233,19 @@ func (c *Context) waitForPlayingPlayers() bool { return !c.disposed } +// dispose makes the goroutine updating the players exit, and waits for it to exit. +func (c *Context) dispose() { + c.markDisposed() + c.updater.Wait() +} + +func (c *Context) markDisposed() { + c.m.Lock() + defer c.m.Unlock() + c.disposed = true + c.cond.Broadcast() +} + func (c *Context) addPlayingPlayer(p *playerImpl) { c.m.Lock() defer c.m.Unlock() diff --git a/audio/audio_test.go b/audio/audio_test.go index fb858ed..73e9517 100644 --- a/audio/audio_test.go +++ b/audio/audio_test.go @@ -71,8 +71,7 @@ func TestGC(t *testing.T) { if err := audio.UpdateForTesting(); err != nil { t.Error(err) } - // 200[ms] should be enough for all the bytes to be consumed. The test runs on the fake - // clock of synctest, so this does not take real time. + // 200[ms] should be enough for all the bytes to be consumed. time.Sleep(200 * time.Millisecond) } t.Errorf("time out") @@ -658,8 +657,7 @@ func TestPositionNotGrowingAfterFinished(t *testing.T) { } p.Play() - // Wait until the player finishes its source. The test runs on the fake clock of synctest, so - // the deadline does not take real time. + // Wait until the player finishes its source. var finished bool deadline := time.Now().Add(10 * time.Second) for time.Now().Before(deadline) { @@ -889,7 +887,6 @@ func TestPlayOnIdleContext(t *testing.T) { } // waitUntil polls cond until it holds or the deadline passes, and reports whether it held. - // The test runs on the fake clock of synctest, so the deadline does not take real time. waitUntil := func(cond func() bool) bool { deadline := time.Now().Add(10 * time.Second) for time.Now().Before(deadline) { diff --git a/audio/export_test.go b/audio/export_test.go index e2638ee..82abad6 100644 --- a/audio/export_test.go +++ b/audio/export_test.go @@ -73,17 +73,20 @@ func (c *dummyContext) NewPlayer(r io.Reader) player { // stopPlayers stops the players' goroutines reading their sources, and waits for them to exit. func (c *dummyContext) stopPlayers() { - c.mu.Lock() - players := c.players - c.players = nil - c.mu.Unlock() - - for _, p := range players { + for _, p := range c.takePlayers() { p.PauseAndStopReading() p.readers.Wait() } } +func (c *dummyContext) takePlayers() []*dummyPlayer { + c.mu.Lock() + defer c.mu.Unlock() + players := c.players + c.players = nil + return players +} + func (c *dummyContext) MaxBufferSize() int { return 48000 * channelCount * bitDepthInBytesInt16 / 4 } @@ -349,23 +352,21 @@ func ContextCreatedForTesting() bool { } // ResetContextForTesting discards the current context, so that a test can create a new one. -// ResetContextForTesting waits for the goroutines the context and its players started to exit, so -// that they do not outlive the test. +// ResetContextForTesting stops the goroutine updating the players and the goroutines reading the +// players' sources, and waits for them to exit. func ResetContextForTesting() { + if c := takeContextForTesting(); c != nil { + c.dispose() + } + dummyContextForTesting.stopPlayers() +} + +func takeContextForTesting() *Context { theContextLock.Lock() + defer theContextLock.Unlock() c := theContext theContext = nil - theContextLock.Unlock() - - if c != nil { - c.m.Lock() - c.disposed = true - c.cond.Broadcast() - c.m.Unlock() - c.updater.Wait() - } - - dummyContextForTesting.stopPlayers() + return c } func (i *InfiniteLoop) SetNoBlendForTesting(value bool) {