Repository navigation
Conversation
0adde71 to
8f34477
Compare
8f34477 to
076c470
Compare
076c470 to
140eff6
Compare
|
the gateway changes look good to me (heartbeat cancel set before starting the goroutine, heartbeat times, nonce, seq/ssrc). build, vet and there's a regression in the audio part though. with type countingProvider struct{ closes atomic.Int32 }
func (*countingProvider) ProvideOpusFrame() ([]byte, error) { return nil, nil }
func (p *countingProvider) Close() { p.closes.Add(1) }
func TestSenderClosesProviderOnce(t *testing.T) {
p := &countingProvider{}
s := NewAudioSender(slog.Default(), p, &connImpl{dave: notReadyDave{}}) // Ready() returns false
s.Open()
time.Sleep(50 * time.Millisecond)
s.Close()
time.Sleep(50 * time.Millisecond)
if n := p.closes.Load(); n != 1 {
t.Fatalf("provider closed %d times", n)
}
}this passes on master and fails here with 2 closes. that can break providers whose with a nil provider ( #604 makes the same one minor thing: |
|
Thanks for the review. Pushed 75309eb:
|
|
looks good now. also tested this together with #604 on current master. no merge conflicts, and |
|
Thanks for catching that. Fixed the same race in |
|
ci needs to be fixed |
Fixed CI |
Summary
Heartbeat state in both gateways (cancel func, interval, sent/received timestamps, nonce) and the voice gateway's SSRC/seq were accessed from several goroutines without synchronization. This guards them with mutexes, and starts the heartbeat with its cancel func set before the goroutine runs, so a concurrent close can no longer miss it.
Session state is left as is: it's already guarded upstream since #634. The audio sender/receiver are left to #604.
Testing
go test --race ./...go build ./...