From f29a28611b944243f48983b538cfa39d34e299c5 Mon Sep 17 00:00:00 2001 From: Raja Subramanian Date: Tue, 16 Jan 2024 12:15:13 +0530 Subject: [PATCH] Prevent writable race. (#2388) It is possible that onBindAndConnectedChanged gets executed in such a way that `writable` does not have the correct value in some very rare timing case (i. e. case like two executions of the function is racing and one atomic was read on first exeuction and second execution runs and sets `writable` and then first execution completes the sets `writable` to incorrect value based on stale read of first execution). Prevent it by executing under bind lock. --- pkg/sfu/downtrack.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/pkg/sfu/downtrack.go b/pkg/sfu/downtrack.go index 696b3a8c8..a2185063f 100644 --- a/pkg/sfu/downtrack.go +++ b/pkg/sfu/downtrack.go @@ -411,6 +411,7 @@ func (d *DownTrack) Bind(t webrtc.TrackLocalContext) (webrtc.RTPCodecParameters, d.onBinding(nil) } d.bound.Store(true) + d.onBindAndConnectedChange() d.bindLock.Unlock() // Bind is called under RTPSender.mu lock, call the RTPSender.GetParameters in goroutine to avoid deadlock @@ -425,9 +426,7 @@ func (d *DownTrack) Bind(t webrtc.TrackLocalContext) (webrtc.RTPCodecParameters, }() d.forwarder.DetermineCodec(d.codec, d.params.Receiver.HeaderExtensions()) - d.params.Logger.Debugw("downtrack bound") - d.onBindAndConnectedChange() return codec, nil } @@ -435,8 +434,10 @@ func (d *DownTrack) Bind(t webrtc.TrackLocalContext) (webrtc.RTPCodecParameters, // Unbind implements the teardown logic when the track is no longer needed. This happens // because a track has been stopped. func (d *DownTrack) Unbind(_ webrtc.TrackLocalContext) error { + d.bindLock.Lock() d.bound.Store(false) d.onBindAndConnectedChange() + d.bindLock.Unlock() return nil } @@ -1577,10 +1578,12 @@ func (d *DownTrack) handleRTCP(bytes []byte) { } func (d *DownTrack) SetConnected() { + d.bindLock.Lock() if !d.connected.Swap(true) { d.onBindAndConnectedChange() } d.params.Logger.Debugw("downtrack connected") + d.bindLock.Unlock() } // SetActivePaddingOnMuteUpTrack will enable padding on the track when its uptrack is muted.