Reject renderer work after audio shutdown
This commit is contained in:
committed by
Brandon McGinty
parent
1679a37783
commit
28a59832c4
@@ -85,7 +85,7 @@ Priority 1: transport, lifecycle, and correctness
|
|||||||
a cancellation context plus WaitGroup/done channel; serialize Start, Stop,
|
a cancellation context plus WaitGroup/done channel; serialize Start, Stop,
|
||||||
reopen, and Destroy; wait before CaptureCloseDevice.
|
reopen, and Destroy; wait before CaptureCloseDevice.
|
||||||
|
|
||||||
10. Renderer can be used after it is closed
|
[x] 10. Renderer can be used after it is closed
|
||||||
Files: gumble/gumbleopenal/stream.go, gumble/gumble/audiolisteners.go
|
Files: gumble/gumbleopenal/stream.go, gumble/gumble/audiolisteners.go
|
||||||
Existing OnAudioStream goroutines can run cleanup after Destroy closes
|
Existing OnAudioStream goroutines can run cleanup after Destroy closes
|
||||||
renderCh. Their final render call then panics sending on a closed channel.
|
renderCh. Their final render call then panics sending on a closed channel.
|
||||||
|
|||||||
@@ -61,10 +61,12 @@ type Stream struct {
|
|||||||
micVolume atomic.Uint32 // float32 stored as bits
|
micVolume atomic.Uint32 // float32 stored as bits
|
||||||
sourceStop chan bool
|
sourceStop chan bool
|
||||||
|
|
||||||
deviceSink *openal.Device
|
deviceSink *openal.Device
|
||||||
contextSink *openal.Context
|
contextSink *openal.Context
|
||||||
renderCh chan renderCommand
|
renderMu sync.RWMutex
|
||||||
renderDone chan struct{}
|
renderCh chan renderCommand
|
||||||
|
renderDone chan struct{}
|
||||||
|
renderClosed bool
|
||||||
|
|
||||||
noiseProcessor NoiseProcessor
|
noiseProcessor NoiseProcessor
|
||||||
noiseProcessorRight NoiseProcessor
|
noiseProcessorRight NoiseProcessor
|
||||||
@@ -159,8 +161,11 @@ func New(client *gumble.Client, inputDevice *string, outputDevice *string, test
|
|||||||
}
|
}
|
||||||
|
|
||||||
func (s *Stream) startRenderer() {
|
func (s *Stream) startRenderer() {
|
||||||
|
s.renderMu.Lock()
|
||||||
|
s.renderClosed = false
|
||||||
s.renderCh = make(chan renderCommand)
|
s.renderCh = make(chan renderCommand)
|
||||||
s.renderDone = make(chan struct{})
|
s.renderDone = make(chan struct{})
|
||||||
|
s.renderMu.Unlock()
|
||||||
ready := make(chan struct{})
|
ready := make(chan struct{})
|
||||||
go func() {
|
go func() {
|
||||||
runtime.LockOSThread()
|
runtime.LockOSThread()
|
||||||
@@ -177,11 +182,18 @@ func (s *Stream) startRenderer() {
|
|||||||
<-ready
|
<-ready
|
||||||
}
|
}
|
||||||
|
|
||||||
// render executes fn on the sole OS thread that owns the OpenAL context.
|
// render executes fn on the sole OS thread that owns the OpenAL context. It
|
||||||
func (s *Stream) render(fn func()) {
|
// returns false after shutdown instead of sending to a closed renderer channel.
|
||||||
|
func (s *Stream) render(fn func()) bool {
|
||||||
|
s.renderMu.RLock()
|
||||||
|
defer s.renderMu.RUnlock()
|
||||||
|
if s.renderClosed || s.renderCh == nil {
|
||||||
|
return false
|
||||||
|
}
|
||||||
command := renderCommand{fn: fn, done: make(chan struct{})}
|
command := renderCommand{fn: fn, done: make(chan struct{})}
|
||||||
s.renderCh <- command
|
s.renderCh <- command
|
||||||
<-command.done
|
<-command.done
|
||||||
|
return true
|
||||||
}
|
}
|
||||||
|
|
||||||
func (s *Stream) AttachStream(client *gumble.Client) {
|
func (s *Stream) AttachStream(client *gumble.Client) {
|
||||||
@@ -296,7 +308,12 @@ func (s *Stream) Destroy() {
|
|||||||
openal.NullContext.Activate()
|
openal.NullContext.Activate()
|
||||||
s.contextSink.Destroy()
|
s.contextSink.Destroy()
|
||||||
})
|
})
|
||||||
close(s.renderCh)
|
s.renderMu.Lock()
|
||||||
|
if !s.renderClosed {
|
||||||
|
close(s.renderCh)
|
||||||
|
s.renderClosed = true
|
||||||
|
}
|
||||||
|
s.renderMu.Unlock()
|
||||||
<-s.renderDone
|
<-s.renderDone
|
||||||
s.contextSink = nil
|
s.contextSink = nil
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -0,0 +1,16 @@
|
|||||||
|
package gumbleopenal
|
||||||
|
|
||||||
|
import "testing"
|
||||||
|
|
||||||
|
// Regression: audio cleanup could send a final render command after Destroy
|
||||||
|
// had closed renderCh, panicking instead of safely discarding that work.
|
||||||
|
func TestRenderRejectsWorkAfterShutdown(t *testing.T) {
|
||||||
|
s := &Stream{renderClosed: true}
|
||||||
|
called := false
|
||||||
|
if s.render(func() { called = true }) {
|
||||||
|
t.Fatal("closed renderer accepted work")
|
||||||
|
}
|
||||||
|
if called {
|
||||||
|
t.Fatal("closed renderer executed work")
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user