Check for closed receiver before setting up rid #3290
Conversation
rtpreceiver.go
Outdated
| r.mu.Lock() | ||
| defer r.mu.Unlock() | ||
|
|
||
| select { |
There was a problem hiding this comment.
Can you use haveClosed instead?
| rtcpReadStream *srtp.ReadStreamSRTCP, | ||
| rtcpInterceptor interceptor.RTCPReader, | ||
| ) error { | ||
| r.mu.Lock() |
There was a problem hiding this comment.
Can you use haveClosed instead?
| } | ||
|
|
||
| if rsid != "" { | ||
| receiver.mu.Lock() |
There was a problem hiding this comment.
If you use the atomic do you still need locking changes?
There was a problem hiding this comment.
Yeah, I just moved the lock inside. Yeah, there are more internal state like tracks in RTPReceiver which needs locking. Not sure why the lock was not internal and called from peerconnection.go. Seemed like a good thing to move it inside.
|
@boks1971 is it possible to get a test/reproduce for this behavior? |
boks1971
force-pushed
the
raja_closed_receiver
branch
2 times, most recently
from
a5ebe77 to
74e9cb7
Compare
Unfortunately, I do not have a specific repro. Happens in production in a larg(ish) call (40 - 50 participants) and they are subscribing to 60 - 80 tracks each and they are connecting from possibly poorer networks. I have been trying to get a definitive proof, but also looking all around for possible places. This one seemed like one possible option as a lot of objects created by buffer factory are leaking. |
boks1971
force-pushed
the
raja_closed_receiver
branch
from
74e9cb7 to
33bc145
Compare
boks1971
force-pushed
the
raja_closed_receiver
branch
from
33bc145 to
29bf0af
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3290 +/- ##
==========================================
- Coverage 84.31% 84.18% -0.13%
==========================================
Files 80 80
Lines 9141 9155 +14
==========================================
Hits 7707 7707
- Misses 1017 1025 +8
- Partials 417 423 +6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
No description provided.