Closed Bug 897981 Opened 6 years ago Closed 6 years ago
Missing lock in Received
RTPPacket/Received RTCPPacket in upstream webrtc .org code
vie_channel.cc locks most RTP/RTCP operations with the rtp_rtcp_cs critical section, as they can get created and stopped (and modified) on different threads, and different threads than data arrives on. However, ReceivedRTPPacket/ReceivedRTCPPacket don't, and when these filter down to the same-named calls in the ViEReceiver class they access receiving_ with no lock, which means there's a potential TSAN bug between reads on the packet arrival thread and shutting down on another thread (in our case MainThread). Note that ViEChannel::StopSend/StopReceive grab the rtp_rtcp_cs lock, and ViEReceiver has it's own receiver_cs, but does not use it for either StopSend/StopReceive or for ReceivedRTPPacket/ReceivedRTCPPacket. (Note that we touched this code in bug 832579, but that didn't change the problem here.) This is an upstream bug and an Issue will need to be filed.
we could also fold InsertRTP/RTCPPacket into ReceivedRTP/RTCPPacket since there are now other callers to them, but since they're separate in upstream I've left it that way. No perf impact as I was able to put the test in an already-existing critical section lock (for incoming packets).
They committed the fix in r4410.
Comment on attachment 781104 [details] [diff] [review] access ViEReceiver::receiving_/receiving_rtcp_ under lock Already in upstream (reviewed by mflodman)
Attachment #781104 - Flags: review?(tterribe)
Comment on attachment 781104 [details] [diff] [review] access ViEReceiver::receiving_/receiving_rtcp_ under lock In upstream, marking as r=flodman :-)
Attachment #781104 - Flags: review?(tterribe) → review+
Target Milestone: --- → mozilla26
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.