Add protection for RTCPSender::max_packet_size_.

webrtc/modules/rtp_rtcp/source/rtcp_sender.cc

 /*
  *  Copyright (c) 2012 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 264 matching lines @@
 void RTCPSender::SetTMMBRStatus(bool enable) {
   rtc::CritScope lock(&critical_section_rtcp_sender_);
   if (enable) {
     SetFlag(RTCPPacketType::kRtcpTmmbr, false);
   } else {
     ConsumeFlag(RTCPPacketType::kRtcpTmmbr, true);
   }
 }
 
 void RTCPSender::SetMaxRtpPacketSize(size_t max_packet_size) {
+  rtc::CritScope lock(&critical_section_rtcp_sender_);
   max_packet_size_ = max_packet_size;
 }
 
 void RTCPSender::SetTimestampOffset(uint32_t timestamp_offset) {
   rtc::CritScope lock(&critical_section_rtcp_sender_);
   timestamp_offset_ = timestamp_offset;
 }
 
 void RTCPSender::SetLastRtpTime(uint32_t rtp_timestamp,
                                 int64_t capture_time_ms) {
@@ skipping 447 matching lines @@
       nack_size, nack_list, pictureID);
 }
 
 int32_t RTCPSender::SendCompoundRTCP(
     const FeedbackState& feedback_state,
     const std::set<RTCPPacketType>& packet_types,
     int32_t nack_size,
     const uint16_t* nack_list,
     uint64_t pictureID) {
   PacketContainer container(transport_, event_log_);
+  size_t max_packet_size;
+
   {
     rtc::CritScope lock(&critical_section_rtcp_sender_);
     if (method_ == RtcpMode::kOff) {
       LOG(LS_WARNING) << "Can't send rtcp if it is disabled.";
       return -1;
     }
     // Add all flags as volatile. Non volatile entries will not be overwritten.
     // All new volatile flags added will be consumed by the end of this call.
     SetFlags(packet_types, true);
 
@@ skipping 52 matching lines @@
     if (packet_bye) {
       container.Append(packet_bye.release());
     }
 
     if (packet_type_counter_observer_ != nullptr) {
       packet_type_counter_observer_->RtcpPacketTypesCounterUpdated(
           remote_ssrc_, packet_type_counter_);
     }
 
     RTC_DCHECK(AllVolatileFlagsConsumed());
+    max_packet_size = max_packet_size_;
   }
 
-  size_t bytes_sent = container.SendPackets(max_packet_size_);
+  size_t bytes_sent = container.SendPackets(max_packet_size);

[nisse-webrtc, 2017/02/21 09:07:26]

Or can we move this inside the above scope, which is protected by the critical
section?

[danilchap, 2017/02/21 09:16:31]

Better not: SendPackets involves calling transport 
that is better to do outside lock.

   return bytes_sent == 0 ? -1 : 0;
 }
 
 void RTCPSender::PrepareReport(const FeedbackState& feedback_state) {
   bool generate_report;
   if (IsFlagPresent(kRtcpSr) || IsFlagPresent(kRtcpRr)) {
     // Report type already explicitly set, don't automatically populate.
     generate_report = true;
     RTC_DCHECK(ConsumeFlag(kRtcpReport) == false);
   } else {
@@ skipping 196 matching lines @@
     }
 
     Transport* const transport_;
     RtcEventLog* const event_log_;
     bool send_failure_;
     // TODO(terelius): We would like to
     // RTC_DISALLOW_IMPLICIT_CONSTRUCTORS(Sender);
     // but we can't because of an incorrect warning (C4822) in MVS 2013.
   } sender(transport_, event_log_);
 
+  size_t max_packet_size;
   {
     rtc::CritScope lock(&critical_section_rtcp_sender_);
     if (method_ == RtcpMode::kOff)
       return false;
+    max_packet_size = max_packet_size_;
   }
 
-  RTC_DCHECK_LE(max_packet_size_, IP_PACKET_SIZE);
+  RTC_DCHECK_LE(max_packet_size, IP_PACKET_SIZE);
   uint8_t buffer[IP_PACKET_SIZE];
-  return packet.BuildExternalBuffer(buffer, max_packet_size_, &sender) &&
+  return packet.BuildExternalBuffer(buffer, max_packet_size, &sender) &&

[nisse-webrtc, 2017/02/21 09:07:26]

Same here, can we move this inside the above, protected, scope?

[danilchap, 2017/02/21 09:16:31]

same here: calling BuildExternalBuffer would call OnPacketReady that calls both
transport and event_log. those operations can be unknownly long, so better to do
it outside lock.

          !sender.send_failure_;
 }
 
 }  // namespace webrtc