Avoid race in VideoReceiveStream shutdown

webrtc/modules/video_coding/jitter_buffer.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.
  */
 #include "webrtc/modules/video_coding/jitter_buffer.h"
@@ skipping 211 matching lines @@
     : clock_(clock),
       running_(false),
       crit_sect_(CriticalSectionWrapper::CreateCriticalSection()),
       frame_event_(std::move(event)),
       max_number_of_frames_(kStartNumberOfFrames),
       free_frames_(),
       decodable_frames_(),
       incomplete_frames_(),
       last_decoded_state_(),
       first_packet_since_reset_(true),
-      stats_callback_(NULL),
+      stats_callback_(nullptr),
       incoming_frame_rate_(0),
       incoming_frame_count_(0),
       time_last_incoming_frame_count_(0),
       incoming_bit_count_(0),
       incoming_bit_rate_(0),
       num_consecutive_old_packets_(0),
       num_packets_(0),
       num_duplicated_packets_(0),
       num_discarded_packets_(0),
       time_first_packet_ms_(0),
       jitter_estimate_(clock),
       inter_frame_delay_(clock_->TimeInMilliseconds()),
       rtt_ms_(kDefaultRtt),
       nack_mode_(kNoNack),
       low_rtt_nack_threshold_ms_(-1),
       high_rtt_nack_threshold_ms_(-1),
       missing_sequence_numbers_(SequenceNumberLessThan()),
+      latest_received_sequence_number_(0),
       max_nack_list_size_(0),
       max_packet_age_to_nack_(0),
       max_incomplete_time_ms_(0),
       decode_error_mode_(kNoErrors),
       average_packets_per_frame_(0.0f),
       frame_counter_(0) {
   for (int i = 0; i < kStartNumberOfFrames; i++)
     free_frames_.push_back(new VCMFrameBuffer());
 }
 
@@ skipping 58 matching lines @@
   num_discarded_packets_ = 0;
   time_first_packet_ms_ = 0;
 
   // Start in a non-signaled state.
   waiting_for_completion_.frame_size = 0;
   waiting_for_completion_.timestamp = 0;
   waiting_for_completion_.latest_packet_time = -1;
   first_packet_since_reset_ = true;
   rtt_ms_ = kDefaultRtt;
   last_decoded_state_.Reset();
+
+  decodable_frames_.Reset(&free_frames_);
+  incomplete_frames_.Reset(&free_frames_);
 }
 
 void VCMJitterBuffer::Stop() {
-  crit_sect_->Enter();
-  UpdateHistograms();
-  running_ = false;
-  last_decoded_state_.Reset();
+  {

[pbos-webrtc, 2016/07/13 16:43:05]

Remove the {}s, the time for calling event->set() should be negligible.

[sprang_webrtc, 2016/07/13 16:51:32]

Done.

+    CriticalSectionScoped cs(crit_sect_);
+    UpdateHistograms();
+    running_ = false;
 
-  // Make sure all frames are free and reset.
-  for (FrameList::iterator it = decodable_frames_.begin();
-       it != decodable_frames_.end(); ++it) {
-    free_frames_.push_back(it->second);
+    last_decoded_state_.Reset();
   }
-  for (FrameList::iterator it = incomplete_frames_.begin();
-       it != incomplete_frames_.end(); ++it) {
-    free_frames_.push_back(it->second);
-  }
-  for (UnorderedFrameList::iterator it = free_frames_.begin();
-       it != free_frames_.end(); ++it) {
-    (*it)->Reset();
-  }
-  decodable_frames_.clear();
-  incomplete_frames_.clear();
-  crit_sect_->Leave();
+
   // Make sure we wake up any threads waiting on these events.
   frame_event_->Set();
 }
 
 bool VCMJitterBuffer::Running() const {
   CriticalSectionScoped cs(crit_sect_);
   return running_;
 }
 
 void VCMJitterBuffer::Flush() {
@@ skipping 225 matching lines @@
 
   if ((*frame).IsSessionComplete())
     UpdateAveragePacketsPerFrame(frame->NumPackets());
 
   return frame;
 }
 
 // Release frame when done with decoding. Should never be used to release
 // frames from within the jitter buffer.
 void VCMJitterBuffer::ReleaseFrame(VCMEncodedFrame* frame) {
+  RTC_CHECK(frame != nullptr);
   CriticalSectionScoped cs(crit_sect_);
   VCMFrameBuffer* frame_buffer = static_cast<VCMFrameBuffer*>(frame);
-  if (frame_buffer) {
-    free_frames_.push_back(frame_buffer);
-  }
+  RecycleFrameBuffer(frame_buffer);
 }
 
 // Gets frame to use for this timestamp. If no match, get empty frame.
 VCMFrameBufferEnum VCMJitterBuffer::GetFrame(const VCMPacket& packet,
                                              VCMFrameBuffer** frame,
                                              FrameList** frame_list) {
   *frame = incomplete_frames_.PopFrame(packet.timestamp);
   if (*frame != NULL) {
     *frame_list = &incomplete_frames_;
     return kNoError;
   }
   *frame = decodable_frames_.PopFrame(packet.timestamp);
   if (*frame != NULL) {
     *frame_list = &decodable_frames_;
     return kNoError;
   }
 
   *frame_list = NULL;
   // No match, return empty frame.
   *frame = GetEmptyFrame();
   if (*frame == NULL) {
     // No free frame! Try to reclaim some...
     LOG(LS_WARNING) << "Unable to get empty frame; Recycling.";
     bool found_key_frame = RecycleFramesUntilKeyFrame();
     *frame = GetEmptyFrame();
-    assert(*frame);
+    RTC_CHECK(*frame);
     if (!found_key_frame) {
-      free_frames_.push_back(*frame);
+      RecycleFrameBuffer(*frame);
       return kFlushIndicator;
     }
   }
   (*frame)->Reset();
   return kNoError;
 }
 
 int64_t VCMJitterBuffer::LastPacketTime(const VCMEncodedFrame* frame,
                                         bool* retransmitted) const {
   assert(retransmitted);
@@ skipping 106 matching lines @@
           LatestSequenceNumber(latest_received_sequence_number_, packet.seqNum);
     }
   }
 
   // Is the frame already in the decodable list?
   bool continuous = IsContinuous(*frame);
   switch (buffer_state) {
     case kGeneralError:
     case kTimeStampError:
     case kSizeError: {
-      free_frames_.push_back(frame);
+      RecycleFrameBuffer(frame);
       break;
     }
     case kCompleteSession: {
       if (previous_state != kStateDecodable &&
           previous_state != kStateComplete) {
         CountFrame(*frame);
         if (continuous) {
           // Signal that we have a complete session.
           frame_event_->Set();
         }
@@ skipping 13 matching lines @@
             NonContinuousOrIncompleteDuration() >
                 90 * kMaxDiscontinuousFramesTime) {
           return kFlushIndicator;
         }
       }
       break;
     }
     case kIncomplete: {
       if (frame->GetState() == kStateEmpty &&
           last_decoded_state_.UpdateEmptyFrame(frame)) {
-        free_frames_.push_back(frame);
+        RecycleFrameBuffer(frame);
         return kNoError;
       } else {
         incomplete_frames_.InsertFrame(frame);
         // If NACKs are enabled, keyframes are triggered by |GetNackList|.
         if (nack_mode_ == kNoNack &&
             NonContinuousOrIncompleteDuration() >
                 90 * kMaxDiscontinuousFramesTime) {
           return kFlushIndicator;
         }
       }
       break;
     }
     case kNoError:
     case kOutOfBoundsPacket:
     case kDuplicatePacket: {
       // Put back the frame where it came from.
       if (frame_list != NULL) {
         frame_list->InsertFrame(frame);
       } else {
-        free_frames_.push_back(frame);
+        RecycleFrameBuffer(frame);
       }
       ++num_duplicated_packets_;
       break;
     }
     case kFlushIndicator:
-      free_frames_.push_back(frame);
+      RecycleFrameBuffer(frame);
       return kFlushIndicator;
     default:
       assert(false);
   }
   return buffer_state;
 }
 
 bool VCMJitterBuffer::IsContinuousInState(
     const VCMFrameBuffer& frame,
     const VCMDecodingState& decoding_state) const {
@@ skipping 481 matching lines @@
   }
   // Evaluate if the RTT is higher than |high_rtt_nack_threshold_ms_|, and in
   // that case we don't wait for retransmissions.
   if (high_rtt_nack_threshold_ms_ >= 0 &&
       rtt_ms_ >= high_rtt_nack_threshold_ms_) {
     return false;
   }
   return true;
 }
 
+void VCMJitterBuffer::RecycleFrameBuffer(VCMFrameBuffer* frame) {
+  frame->Reset();
+  free_frames_.push_back(frame);
+}
+
 }  // namespace webrtc