Synchronize UDP decoding with client state
This commit is contained in:
committed by
Brandon McGinty
parent
0766178277
commit
7180b7604d
@@ -16,7 +16,7 @@ Priority 0: security and crashers
|
|||||||
voice traffic. Remove all secret material from logs. At most log lengths,
|
voice traffic. Remove all secret material from logs. At most log lengths,
|
||||||
setup success, and a non-secret connection identifier.
|
setup success, and a non-secret connection identifier.
|
||||||
|
|
||||||
2. Native UDP races TCP state mutation
|
[x] 2. Native UDP races TCP state mutation
|
||||||
Files: gumble/gumble/udp15.go, gumble/gumble/handlers.go,
|
Files: gumble/gumble/udp15.go, gumble/gumble/handlers.go,
|
||||||
gumble/gumble/audiolisteners.go
|
gumble/gumble/audiolisteners.go
|
||||||
The UDP reader runs independently of the TCP read routine. It reads
|
The UDP reader runs independently of the TCP read routine. It reads
|
||||||
|
|||||||
@@ -85,6 +85,10 @@ func (c *Client) handleVersion(buffer []byte) error {
|
|||||||
}
|
}
|
||||||
|
|
||||||
func (c *Client) handleUDPTunnel(buffer []byte) error {
|
func (c *Client) handleUDPTunnel(buffer []byte) error {
|
||||||
|
// Native UDP and TCP tunnel packets can arrive concurrently. Keep the user
|
||||||
|
// map and its decoder/sequence state stable for the entire decode.
|
||||||
|
c.volatile.RLock()
|
||||||
|
defer c.volatile.RUnlock()
|
||||||
if len(buffer) < 1 {
|
if len(buffer) < 1 {
|
||||||
log.Warn("handleUDPTunnel: empty buffer")
|
log.Warn("handleUDPTunnel: empty buffer")
|
||||||
return errInvalidProtobuf
|
return errInvalidProtobuf
|
||||||
|
|||||||
@@ -704,6 +704,9 @@ func (c *Client) markUDPActive() {
|
|||||||
// dispatchOpus15 processes a decoded MumbleUDP.Audio frame and dispatches
|
// dispatchOpus15 processes a decoded MumbleUDP.Audio frame and dispatches
|
||||||
// the decoded PCM to audio listeners.
|
// the decoded PCM to audio listeners.
|
||||||
func (c *Client) dispatchOpus15(pktNum uint64, session uint32, frameNum int64, opusData []byte, terminator bool, context uint32, position *[3]float32, volumeAdjustment float32) {
|
func (c *Client) dispatchOpus15(pktNum uint64, session uint32, frameNum int64, opusData []byte, terminator bool, context uint32, position *[3]float32, volumeAdjustment float32) {
|
||||||
|
// This runs on the UDP reader independently of TCP state handlers.
|
||||||
|
c.volatile.RLock()
|
||||||
|
defer c.volatile.RUnlock()
|
||||||
if len(opusData) == 0 && !terminator {
|
if len(opusData) == 0 && !terminator {
|
||||||
log.Info("UDP15 #%d: no opus data (session=%d frame=%d), skipping", pktNum, session, frameNum)
|
log.Info("UDP15 #%d: no opus data (session=%d frame=%d), skipping", pktNum, session, frameNum)
|
||||||
return
|
return
|
||||||
|
|||||||
@@ -0,0 +1,27 @@
|
|||||||
|
package gumble
|
||||||
|
|
||||||
|
import (
|
||||||
|
"testing"
|
||||||
|
"time"
|
||||||
|
)
|
||||||
|
|
||||||
|
// Regression: native UDP decoded Users and per-user decoder state while TCP
|
||||||
|
// handlers concurrently removed users or changed channels. UDP decoding must
|
||||||
|
// share the client state lock with those handlers.
|
||||||
|
func TestUDPTunnelWaitsForClientStateLock(t *testing.T) {
|
||||||
|
c := &Client{}
|
||||||
|
c.volatile.Lock()
|
||||||
|
done := make(chan struct{})
|
||||||
|
go func() { _ = c.handleUDPTunnel([]byte{0}); close(done) }()
|
||||||
|
select {
|
||||||
|
case <-done:
|
||||||
|
t.Fatal("UDP handler bypassed client state lock")
|
||||||
|
case <-time.After(20 * time.Millisecond):
|
||||||
|
}
|
||||||
|
c.volatile.Unlock()
|
||||||
|
select {
|
||||||
|
case <-done:
|
||||||
|
case <-time.After(time.Second):
|
||||||
|
t.Fatal("UDP handler did not resume")
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user