Fix race condition in FrameBuffer2

webrtc/modules/video_coding/frame_buffer2.cc

 /*
  *  Copyright (c) 2016 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 36 matching lines @@
       stopped_(false),
       protection_mode_(kProtectionNack) {}
 
 FrameBuffer::~FrameBuffer() {
   UpdateHistograms();
 }
 
 FrameBuffer::ReturnReason FrameBuffer::NextFrame(
     int64_t max_wait_time_ms,
     std::unique_ptr<FrameObject>* frame_out) {
-  int64_t latest_return_time = clock_->TimeInMilliseconds() + max_wait_time_ms;
+  int64_t latest_return_time_ms =
+      clock_->TimeInMilliseconds() + max_wait_time_ms;
   int64_t wait_ms = max_wait_time_ms;
-  FrameMap::iterator next_frame_it;
 
   do {
     int64_t now_ms = clock_->TimeInMilliseconds();
     {
       rtc::CritScope lock(&crit_);
       new_countinuous_frame_event_.Reset();
       if (stopped_)
         return kStopped;
 
       wait_ms = max_wait_time_ms;
 
       // Need to hold |crit_| in order to use |frames_|, therefore we
       // set it here in the loop instead of outside the loop in order to not
       // acquire the lock unnecesserily.
-      next_frame_it = frames_.end();
+      next_frame_it_ = frames_.end();
 
       // |frame_it| points to the first frame after the
       // |last_decoded_frame_it_|.
       auto frame_it = frames_.end();
       if (last_decoded_frame_it_ == frames_.end()) {
         frame_it = frames_.begin();
       } else {
         frame_it = last_decoded_frame_it_;
         ++frame_it;
       }
 
       // |continuous_end_it| points to the first frame after the
       // |last_continuous_frame_it_|.
       auto continuous_end_it = last_continuous_frame_it_;
       if (continuous_end_it != frames_.end())
         ++continuous_end_it;
 
       for (; frame_it != continuous_end_it; ++frame_it) {
         if (!frame_it->second.continuous ||
             frame_it->second.num_missing_decodable > 0) {
           continue;
         }
 
         FrameObject* frame = frame_it->second.frame.get();
-        next_frame_it = frame_it;
+        next_frame_it_ = frame_it;
         if (frame->RenderTime() == -1)
           frame->SetRenderTime(timing_->RenderTimeMs(frame->timestamp, now_ms));
         wait_ms = timing_->MaxWaitingTime(frame->RenderTime(), now_ms);
 
         // This will cause the frame buffer to prefer high framerate rather
         // than high resolution in the case of the decoder not decoding fast
         // enough and the stream has multiple spatial and temporal layers.
         if (wait_ms == 0)
           continue;
 
         break;
       }
     }  // rtc::Critscope lock(&crit_);
 
-    wait_ms = std::min<int64_t>(wait_ms, latest_return_time - now_ms);
+    wait_ms = std::min<int64_t>(wait_ms, latest_return_time_ms - now_ms);
     wait_ms = std::max<int64_t>(wait_ms, 0);
   } while (new_countinuous_frame_event_.Wait(wait_ms));
 
   rtc::CritScope lock(&crit_);
-  if (next_frame_it != frames_.end()) {
-    std::unique_ptr<FrameObject> frame = std::move(next_frame_it->second.frame);
+  int64_t now_ms = clock_->TimeInMilliseconds();
+  if (next_frame_it_ != frames_.end()) {
+    std::unique_ptr<FrameObject> frame =
+        std::move(next_frame_it_->second.frame);
 
     if (!frame->delayed_by_retransmission()) {
       int64_t frame_delay;
 
       if (inter_frame_delay_.CalculateDelay(frame->timestamp, &frame_delay,
                                             frame->ReceivedTime())) {
         jitter_estimator_->UpdateEstimate(frame_delay, frame->size());
       }
 
       float rtt_mult = protection_mode_ == kProtectionNackFEC ? 0.0 : 1.0;
       timing_->SetJitterDelay(jitter_estimator_->GetJitterEstimate(rtt_mult));
-      timing_->UpdateCurrentDelay(frame->RenderTime(),
-                                  clock_->TimeInMilliseconds());
+      timing_->UpdateCurrentDelay(frame->RenderTime(), now_ms);
     }
 
     UpdateJitterDelay();
 
-    PropagateDecodability(next_frame_it->second);
-    AdvanceLastDecodedFrame(next_frame_it);
+    PropagateDecodability(next_frame_it_->second);
+    AdvanceLastDecodedFrame(next_frame_it_);
     last_decoded_frame_timestamp_ = frame->timestamp;
     *frame_out = std::move(frame);
     return kFrameFound;
+  } else if (latest_return_time_ms - now_ms > 0) {
+    // If |next_frame_it_ == frames_.end()| and there is still time left, it
+    // means that the frame buffer was cleared as the thread in this function
+    // was waiting to acquire |crit_| in order to return. Wait for the
+    // remaining time and then return.
+    return NextFrame(latest_return_time_ms - now_ms, frame_out);
   } else {
     return kTimeout;
   }
 }
 
 void FrameBuffer::SetProtectionMode(VCMVideoProtection mode) {
   rtc::CritScope lock(&crit_);
   protection_mode_ = mode;
 }
 
@@ skipping 250 matching lines @@
   if (accumulated_delay_samples_ > 0) {
     RTC_HISTOGRAM_COUNTS_10000("WebRTC.Video.JitterBufferDelayInMs",
                                accumulated_delay_ / accumulated_delay_samples_);
   }
 }
 
 void FrameBuffer::ClearFramesAndHistory() {
   frames_.clear();
   last_decoded_frame_it_ = frames_.end();
   last_continuous_frame_it_ = frames_.end();
+  next_frame_it_ = frames_.end();
   num_frames_history_ = 0;
   num_frames_buffered_ = 0;
 }
 
 }  // namespace video_coding
 }  // namespace webrtc