Avoid data race in RtcpReceiver.

See eg https://build.chromium.org/p/client.webrtc/builders/Linux%20Tsan%20v2/builds/3930/steps/video_engine_tests/logs/stdio

Also some cleanup, lock annotations.

BUG=

Review URL: https://codereview.webrtc.org/1401463003

Cr-Commit-Position: refs/heads/master@{#10266}

2 files changed, 68 insertions, 60 deletions

diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc b/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc
index fb1f9b2503..abd822a4b9 100644
--- a/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc
+++ b/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc
@@ -387,7 +387,6 @@ RTCPReceiver::IncomingRTCPPacket(RTCPPacketInformation& rtcpPacketInformation,
     return 0;
 }
 
-// no need for critsect we have _criticalSectionRTCPReceiver
 void
 RTCPReceiver::HandleSenderReceiverReport(RTCPUtility::RTCPParserV2& rtcpParser,
                                          RTCPPacketInformation& rtcpPacketInformation)
@@ -463,7 +462,6 @@ RTCPReceiver::HandleSenderReceiverReport(RTCPUtility::RTCPParserV2& rtcpParser,
     }
 }
 
-// no need for critsect we have _criticalSectionRTCPReceiver
 void RTCPReceiver::HandleReportBlock(
     const RTCPUtility::RTCPPacket& rtcpPacket,
     RTCPPacketInformation& rtcpPacketInformation,
@@ -775,7 +773,6 @@ int32_t RTCPReceiver::BoundingSet(bool &tmmbrOwner, TMMBRSet* boundingSetRec) {
   return receiveInfo->TmmbnBoundingSet.lengthOfSet();
 }
 
-// no need for critsect we have _criticalSectionRTCPReceiver
 void RTCPReceiver::HandleSDES(RTCPUtility::RTCPParserV2& rtcpParser,
                               RTCPPacketInformation& rtcpPacketInformation) {
   RTCPUtility::RTCPPacketTypes pktType = rtcpParser.Iterate();
@@ -786,7 +783,6 @@ void RTCPReceiver::HandleSDES(RTCPUtility::RTCPParserV2& rtcpParser,
   rtcpPacketInformation.rtcpPacketTypeFlags |= kRtcpSdes;
 }
 
-// no need for critsect we have _criticalSectionRTCPReceiver
 void RTCPReceiver::HandleSDESChunk(RTCPUtility::RTCPParserV2& rtcpParser) {
   const RTCPUtility::RTCPPacket& rtcpPacket = rtcpParser.Packet();
   RTCPCnameInformation* cnameInfo =
@@ -804,7 +800,6 @@ void RTCPReceiver::HandleSDESChunk(RTCPUtility::RTCPParserV2& rtcpParser) {
   }
 }
 
-// no need for critsect we have _criticalSectionRTCPReceiver
 void RTCPReceiver::HandleNACK(RTCPUtility::RTCPParserV2& rtcpParser,
                               RTCPPacketInformation& rtcpPacketInformation) {
   const RTCPUtility::RTCPPacket& rtcpPacket = rtcpParser.Packet();
@@ -828,7 +823,6 @@ void RTCPReceiver::HandleNACK(RTCPUtility::RTCPParserV2& rtcpParser,
   }
 }
 
-// no need for critsect we have _criticalSectionRTCPReceiver
 void
 RTCPReceiver::HandleNACKItem(const RTCPUtility::RTCPPacket& rtcpPacket,
                              RTCPPacketInformation& rtcpPacketInformation) {
@@ -848,12 +842,10 @@ RTCPReceiver::HandleNACKItem(const RTCPUtility::RTCPPacket& rtcpPacket,
   rtcpPacketInformation.rtcpPacketTypeFlags |= kRtcpNack;
 }
 
-// no need for critsect we have _criticalSectionRTCPReceiver
 void RTCPReceiver::HandleBYE(RTCPUtility::RTCPParserV2& rtcpParser) {
   const RTCPUtility::RTCPPacket& rtcpPacket = rtcpParser.Packet();
 
   // clear our lists
-  CriticalSectionScoped lock(_criticalSectionRTCPReceiver);
   ReportBlockMap::iterator it = _receivedReportBlockMap.begin();
   for (; it != _receivedReportBlockMap.end(); ++it) {
     ReportBlockInfoMap* info_map = &(it->second);
@@ -964,15 +956,12 @@ void RTCPReceiver::HandleXrDlrrReportBlockItem(
   rtcpPacketInformation.rtcpPacketTypeFlags |= kRtcpXrDlrrReportBlock;
 }
 
-// no need for critsect we have _criticalSectionRTCPReceiver
 void
 RTCPReceiver::HandleXRVOIPMetric(RTCPUtility::RTCPParserV2& rtcpParser,
                                  RTCPPacketInformation& rtcpPacketInformation)
 {
     const RTCPUtility::RTCPPacket& rtcpPacket = rtcpParser.Packet();
 
-    CriticalSectionScoped lock(_criticalSectionRTCPReceiver);
-
     if(rtcpPacket.XRVOIPMetricItem.SSRC == main_ssrc_)
     {
         // Store VoIP metrics block if it's about me
@@ -1008,7 +997,6 @@ RTCPReceiver::HandleXRVOIPMetric(RTCPUtility::RTCPParserV2& rtcpParser,
     rtcpParser.Iterate();
 }
 
-// no need for critsect we have _criticalSectionRTCPReceiver
 void RTCPReceiver::HandlePLI(RTCPUtility::RTCPParserV2& rtcpParser,
                              RTCPPacketInformation& rtcpPacketInformation) {
   const RTCPUtility::RTCPPacket& rtcpPacket = rtcpParser.Packet();
@@ -1022,7 +1010,6 @@ void RTCPReceiver::HandlePLI(RTCPUtility::RTCPParserV2& rtcpParser,
   rtcpParser.Iterate();
 }
 
-// no need for critsect we have _criticalSectionRTCPReceiver
 void RTCPReceiver::HandleTMMBR(RTCPUtility::RTCPParserV2& rtcpParser,
                                RTCPPacketInformation& rtcpPacketInformation) {
   const RTCPUtility::RTCPPacket& rtcpPacket = rtcpParser.Packet();
@@ -1059,7 +1046,6 @@ void RTCPReceiver::HandleTMMBR(RTCPUtility::RTCPParserV2& rtcpParser,
   }
 }
 
-// no need for critsect we have _criticalSectionRTCPReceiver
 void RTCPReceiver::HandleTMMBRItem(RTCPReceiveInformation& receiveInfo,
                                    const RTCPUtility::RTCPPacket& rtcpPacket,
                                    RTCPPacketInformation& rtcpPacketInformation,
@@ -1072,7 +1058,6 @@ void RTCPReceiver::HandleTMMBRItem(RTCPReceiveInformation& receiveInfo,
   }
 }
 
-// no need for critsect we have _criticalSectionRTCPReceiver
 void RTCPReceiver::HandleTMMBN(RTCPUtility::RTCPParserV2& rtcpParser,
                                RTCPPacketInformation& rtcpPacketInformation) {
   const RTCPUtility::RTCPPacket& rtcpPacket = rtcpParser.Packet();
@@ -1104,14 +1089,12 @@ void RTCPReceiver::HandleTMMBN(RTCPUtility::RTCPParserV2& rtcpParser,
   }
 }
 
-// no need for critsect we have _criticalSectionRTCPReceiver
 void RTCPReceiver::HandleSR_REQ(RTCPUtility::RTCPParserV2& rtcpParser,
                                 RTCPPacketInformation& rtcpPacketInformation) {
   rtcpPacketInformation.rtcpPacketTypeFlags |= kRtcpSrReq;
   rtcpParser.Iterate();
 }
 
-// no need for critsect we have _criticalSectionRTCPReceiver
 void RTCPReceiver::HandleTMMBNItem(RTCPReceiveInformation& receiveInfo,
                                    const RTCPUtility::RTCPPacket& rtcpPacket) {
   receiveInfo.TmmbnBoundingSet.AddEntry(
@@ -1120,7 +1103,6 @@ void RTCPReceiver::HandleTMMBNItem(RTCPReceiveInformation& receiveInfo,
       rtcpPacket.TMMBNItem.SSRC);
 }
 
-// no need for critsect we have _criticalSectionRTCPReceiver
 void RTCPReceiver::HandleSLI(RTCPUtility::RTCPParserV2& rtcpParser,
                              RTCPPacketInformation& rtcpPacketInformation) {
   const RTCPUtility::RTCPPacket& rtcpPacket = rtcpParser.Packet();
@@ -1131,7 +1113,6 @@ void RTCPReceiver::HandleSLI(RTCPUtility::RTCPParserV2& rtcpParser,
   }
 }
 
-// no need for critsect we have _criticalSectionRTCPReceiver
 void RTCPReceiver::HandleSLIItem(const RTCPUtility::RTCPPacket& rtcpPacket,
                                  RTCPPacketInformation& rtcpPacketInformation) {
   // in theory there could be multiple slices lost
@@ -1167,7 +1148,6 @@ RTCPReceiver::HandleRPSI(RTCPUtility::RTCPParserV2& rtcpParser,
     }
 }
 
-// no need for critsect we have _criticalSectionRTCPReceiver
 void RTCPReceiver::HandlePsfbApp(RTCPUtility::RTCPParserV2& rtcpParser,
                                  RTCPPacketInformation& rtcpPacketInformation) {
   RTCPUtility::RTCPPacketTypes pktType = rtcpParser.Iterate();
@@ -1180,7 +1160,6 @@ void RTCPReceiver::HandlePsfbApp(RTCPUtility::RTCPParserV2& rtcpParser,
   }
 }
 
-// no need for critsect we have _criticalSectionRTCPReceiver
 void RTCPReceiver::HandleIJ(RTCPUtility::RTCPParserV2& rtcpParser,
                             RTCPPacketInformation& rtcpPacketInformation) {
   const RTCPUtility::RTCPPacket& rtcpPacket = rtcpParser.Packet();
@@ -1208,7 +1187,6 @@ void RTCPReceiver::HandleREMBItem(
       rtcpPacket.REMBItem.BitRate;
 }
 
-// no need for critsect we have _criticalSectionRTCPReceiver
 void RTCPReceiver::HandleFIR(RTCPUtility::RTCPParserV2& rtcpParser,
                              RTCPPacketInformation& rtcpPacketInformation) {
   const RTCPUtility::RTCPPacket& rtcpPacket = rtcpParser.Packet();
@@ -1222,7 +1200,6 @@ void RTCPReceiver::HandleFIR(RTCPUtility::RTCPParserV2& rtcpParser,
   }
 }
 
-// no need for critsect we have _criticalSectionRTCPReceiver
 void RTCPReceiver::HandleFIRItem(RTCPReceiveInformation* receiveInfo,
                                  const RTCPUtility::RTCPPacket& rtcpPacket,
                                  RTCPPacketInformation& rtcpPacketInformation) {
@@ -1348,11 +1325,13 @@ void RTCPReceiver::TriggerCallbacksFromRTCPPacket(
     // Might trigger a OnReceivedBandwidthEstimateUpdate.
     UpdateTMMBR();
   }
-  unsigned int local_ssrc;
+  uint32_t local_ssrc;
+  std::set<uint32_t> registered_ssrcs;
   {
     // We don't want to hold this critsect when triggering the callbacks below.
     CriticalSectionScoped lock(_criticalSectionRTCPReceiver);
     local_ssrc = main_ssrc_;
+    registered_ssrcs = registered_ssrcs_;
   }
   if (!receiver_only_ &&
       (rtcpPacketInformation.rtcpPacketTypeFlags & kRtcpSrReq)) {
@@ -1414,9 +1393,8 @@ void RTCPReceiver::TriggerCallbacksFromRTCPPacket(
         (rtcpPacketInformation.rtcpPacketTypeFlags & kRtcpTransportFeedback)) {
       uint32_t media_source_ssrc =
           rtcpPacketInformation.transport_feedback_->GetMediaSourceSsrc();
-      if (media_source_ssrc == main_ssrc_ ||
-          registered_ssrcs_.find(media_source_ssrc) !=
-              registered_ssrcs_.end()) {
+      if (media_source_ssrc == local_ssrc ||
+          registered_ssrcs.find(media_source_ssrc) != registered_ssrcs.end()) {
         _cbTransportFeedbackObserver->OnTransportFeedback(
             *rtcpPacketInformation.transport_feedback_.get());
       }
diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_receiver.h b/webrtc/modules/rtp_rtcp/source/rtcp_receiver.h
index 2883f3af34..272397675b 100644
--- a/webrtc/modules/rtp_rtcp/source/rtcp_receiver.h
+++ b/webrtc/modules/rtp_rtcp/source/rtcp_receiver.h
@@ -124,102 +124,132 @@ protected:
 
     void HandleSenderReceiverReport(
         RTCPUtility::RTCPParserV2& rtcpParser,
-        RTCPHelp::RTCPPacketInformation& rtcpPacketInformation);
+        RTCPHelp::RTCPPacketInformation& rtcpPacketInformation)
+        EXCLUSIVE_LOCKS_REQUIRED(_criticalSectionRTCPReceiver);
 
     void HandleReportBlock(
         const RTCPUtility::RTCPPacket& rtcpPacket,
         RTCPHelp::RTCPPacketInformation& rtcpPacketInformation,
-        uint32_t remoteSSRC);
+        uint32_t remoteSSRC)
+        EXCLUSIVE_LOCKS_REQUIRED(_criticalSectionRTCPReceiver);
 
     void HandleSDES(RTCPUtility::RTCPParserV2& rtcpParser,
-                    RTCPHelp::RTCPPacketInformation& rtcpPacketInformation);
+                    RTCPHelp::RTCPPacketInformation& rtcpPacketInformation)
+        EXCLUSIVE_LOCKS_REQUIRED(_criticalSectionRTCPReceiver);
 
-    void HandleSDESChunk(RTCPUtility::RTCPParserV2& rtcpParser);
+    void HandleSDESChunk(RTCPUtility::RTCPParserV2& rtcpParser)
+        EXCLUSIVE_LOCKS_REQUIRED(_criticalSectionRTCPReceiver);
 
     void HandleXrHeader(RTCPUtility::RTCPParserV2& parser,
-                        RTCPHelp::RTCPPacketInformation& rtcpPacketInformation);
+                        RTCPHelp::RTCPPacketInformation& rtcpPacketInformation)
+        EXCLUSIVE_LOCKS_REQUIRED(_criticalSectionRTCPReceiver);
 
     void HandleXrReceiveReferenceTime(
         RTCPUtility::RTCPParserV2& parser,
-        RTCPHelp::RTCPPacketInformation& rtcpPacketInformation);
+        RTCPHelp::RTCPPacketInformation& rtcpPacketInformation)
+        EXCLUSIVE_LOCKS_REQUIRED(_criticalSectionRTCPReceiver);
 
     void HandleXrDlrrReportBlock(
         RTCPUtility::RTCPParserV2& parser,
-        RTCPHelp::RTCPPacketInformation& rtcpPacketInformation);
+        RTCPHelp::RTCPPacketInformation& rtcpPacketInformation)
+        EXCLUSIVE_LOCKS_REQUIRED(_criticalSectionRTCPReceiver);
 
     void HandleXrDlrrReportBlockItem(
         const RTCPUtility::RTCPPacket& packet,
-        RTCPHelp::RTCPPacketInformation& rtcpPacketInformation);
+        RTCPHelp::RTCPPacketInformation& rtcpPacketInformation)
+        EXCLUSIVE_LOCKS_REQUIRED(_criticalSectionRTCPReceiver);
 
     void HandleXRVOIPMetric(
         RTCPUtility::RTCPParserV2& rtcpParser,
-        RTCPHelp::RTCPPacketInformation& rtcpPacketInformation);
+        RTCPHelp::RTCPPacketInformation& rtcpPacketInformation)
+        EXCLUSIVE_LOCKS_REQUIRED(_criticalSectionRTCPReceiver);
 
     void HandleNACK(RTCPUtility::RTCPParserV2& rtcpParser,
-                    RTCPHelp::RTCPPacketInformation& rtcpPacketInformation);
+                    RTCPHelp::RTCPPacketInformation& rtcpPacketInformation)
+        EXCLUSIVE_LOCKS_REQUIRED(_criticalSectionRTCPReceiver);
 
     void HandleNACKItem(const RTCPUtility::RTCPPacket& rtcpPacket,
-                        RTCPHelp::RTCPPacketInformation& rtcpPacketInformation);
+                        RTCPHelp::RTCPPacketInformation& rtcpPacketInformation)
+        EXCLUSIVE_LOCKS_REQUIRED(_criticalSectionRTCPReceiver);
 
-    void HandleBYE(RTCPUtility::RTCPParserV2& rtcpParser);
+    void HandleBYE(RTCPUtility::RTCPParserV2& rtcpParser)
+        EXCLUSIVE_LOCKS_REQUIRED(_criticalSectionRTCPReceiver);
 
     void HandlePLI(RTCPUtility::RTCPParserV2& rtcpParser,
-                   RTCPHelp::RTCPPacketInformation& rtcpPacketInformation);
+                   RTCPHelp::RTCPPacketInformation& rtcpPacketInformation)
+        EXCLUSIVE_LOCKS_REQUIRED(_criticalSectionRTCPReceiver);
 
     void HandleSLI(RTCPUtility::RTCPParserV2& rtcpParser,
-                   RTCPHelp::RTCPPacketInformation& rtcpPacketInformation);
+                   RTCPHelp::RTCPPacketInformation& rtcpPacketInformation)
+        EXCLUSIVE_LOCKS_REQUIRED(_criticalSectionRTCPReceiver);
 
     void HandleSLIItem(const RTCPUtility::RTCPPacket& rtcpPacket,
-                       RTCPHelp::RTCPPacketInformation& rtcpPacketInformation);
+                       RTCPHelp::RTCPPacketInformation& rtcpPacketInformation)
+        EXCLUSIVE_LOCKS_REQUIRED(_criticalSectionRTCPReceiver);
 
     void HandleRPSI(RTCPUtility::RTCPParserV2& rtcpParser,
-                    RTCPHelp::RTCPPacketInformation& rtcpPacketInformation);
+                    RTCPHelp::RTCPPacketInformation& rtcpPacketInformation)
+        EXCLUSIVE_LOCKS_REQUIRED(_criticalSectionRTCPReceiver);
 
     void HandlePsfbApp(RTCPUtility::RTCPParserV2& rtcpParser,
-                       RTCPHelp::RTCPPacketInformation& rtcpPacketInformation);
+                       RTCPHelp::RTCPPacketInformation& rtcpPacketInformation)
+        EXCLUSIVE_LOCKS_REQUIRED(_criticalSectionRTCPReceiver);
 
     void HandleREMBItem(RTCPUtility::RTCPParserV2& rtcpParser,
-                        RTCPHelp::RTCPPacketInformation& rtcpPacketInformation);
+                        RTCPHelp::RTCPPacketInformation& rtcpPacketInformation)
+        EXCLUSIVE_LOCKS_REQUIRED(_criticalSectionRTCPReceiver);
 
     void HandleIJ(RTCPUtility::RTCPParserV2& rtcpParser,
-                  RTCPHelp::RTCPPacketInformation& rtcpPacketInformation);
+                  RTCPHelp::RTCPPacketInformation& rtcpPacketInformation)
+        EXCLUSIVE_LOCKS_REQUIRED(_criticalSectionRTCPReceiver);
 
     void HandleIJItem(const RTCPUtility::RTCPPacket& rtcpPacket,
-                      RTCPHelp::RTCPPacketInformation& rtcpPacketInformation);
+                      RTCPHelp::RTCPPacketInformation& rtcpPacketInformation)
+        EXCLUSIVE_LOCKS_REQUIRED(_criticalSectionRTCPReceiver);
 
     void HandleTMMBR(RTCPUtility::RTCPParserV2& rtcpParser,
-                     RTCPHelp::RTCPPacketInformation& rtcpPacketInformation);
+                     RTCPHelp::RTCPPacketInformation& rtcpPacketInformation)
+        EXCLUSIVE_LOCKS_REQUIRED(_criticalSectionRTCPReceiver);
 
     void HandleTMMBRItem(RTCPHelp::RTCPReceiveInformation& receiveInfo,
                          const RTCPUtility::RTCPPacket& rtcpPacket,
                          RTCPHelp::RTCPPacketInformation& rtcpPacketInformation,
-                         uint32_t senderSSRC);
+                         uint32_t senderSSRC)
+        EXCLUSIVE_LOCKS_REQUIRED(_criticalSectionRTCPReceiver);
 
     void HandleTMMBN(RTCPUtility::RTCPParserV2& rtcpParser,
-                     RTCPHelp::RTCPPacketInformation& rtcpPacketInformation);
+                     RTCPHelp::RTCPPacketInformation& rtcpPacketInformation)
+        EXCLUSIVE_LOCKS_REQUIRED(_criticalSectionRTCPReceiver);
 
     void HandleSR_REQ(RTCPUtility::RTCPParserV2& rtcpParser,
-                      RTCPHelp::RTCPPacketInformation& rtcpPacketInformation);
+                      RTCPHelp::RTCPPacketInformation& rtcpPacketInformation)
+        EXCLUSIVE_LOCKS_REQUIRED(_criticalSectionRTCPReceiver);
 
     void HandleTMMBNItem(RTCPHelp::RTCPReceiveInformation& receiveInfo,
-                         const RTCPUtility::RTCPPacket& rtcpPacket);
+                         const RTCPUtility::RTCPPacket& rtcpPacket)
+        EXCLUSIVE_LOCKS_REQUIRED(_criticalSectionRTCPReceiver);
 
     void HandleFIR(RTCPUtility::RTCPParserV2& rtcpParser,
-                   RTCPHelp::RTCPPacketInformation& rtcpPacketInformation);
+                   RTCPHelp::RTCPPacketInformation& rtcpPacketInformation)
+        EXCLUSIVE_LOCKS_REQUIRED(_criticalSectionRTCPReceiver);
 
     void HandleFIRItem(RTCPHelp::RTCPReceiveInformation* receiveInfo,
                        const RTCPUtility::RTCPPacket& rtcpPacket,
-                       RTCPHelp::RTCPPacketInformation& rtcpPacketInformation);
+                       RTCPHelp::RTCPPacketInformation& rtcpPacketInformation)
+        EXCLUSIVE_LOCKS_REQUIRED(_criticalSectionRTCPReceiver);
 
     void HandleAPP(RTCPUtility::RTCPParserV2& rtcpParser,
-                   RTCPHelp::RTCPPacketInformation& rtcpPacketInformation);
+                   RTCPHelp::RTCPPacketInformation& rtcpPacketInformation)
+        EXCLUSIVE_LOCKS_REQUIRED(_criticalSectionRTCPReceiver);
 
     void HandleAPPItem(RTCPUtility::RTCPParserV2& rtcpParser,
-                       RTCPHelp::RTCPPacketInformation& rtcpPacketInformation);
+                       RTCPHelp::RTCPPacketInformation& rtcpPacketInformation)
+        EXCLUSIVE_LOCKS_REQUIRED(_criticalSectionRTCPReceiver);
 
     void HandleTransportFeedback(
         RTCPUtility::RTCPParserV2* rtcp_parser,
-        RTCPHelp::RTCPPacketInformation* rtcp_packet_information);
+        RTCPHelp::RTCPPacketInformation* rtcp_packet_information)
+        EXCLUSIVE_LOCKS_REQUIRED(_criticalSectionRTCPReceiver);
 
  private:
   typedef std::map<uint32_t, RTCPHelp::RTCPReceiveInformation*>
@@ -249,9 +279,9 @@ protected:
   TransportFeedbackObserver* const _cbTransportFeedbackObserver;
 
   CriticalSectionWrapper* _criticalSectionRTCPReceiver;
-  uint32_t main_ssrc_;
-  uint32_t _remoteSSRC;
-  std::set<uint32_t> registered_ssrcs_;
+  uint32_t main_ssrc_ GUARDED_BY(_criticalSectionRTCPReceiver);
+  uint32_t _remoteSSRC GUARDED_BY(_criticalSectionRTCPReceiver);
+  std::set<uint32_t> registered_ssrcs_ GUARDED_BY(_criticalSectionRTCPReceiver);
 
   // Received send report
   RTCPSenderInfo _remoteSenderInfo;