Avoid race in VideoReceiveStream shutdown

webrtc/video/video_receive_stream.cc

 /*
  *  Copyright (c) 2013 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 199 matching lines @@
           congestion_controller_->GetRetransmissionRateLimiter()),
       rtp_stream_sync_(&video_receiver_, &rtp_stream_receiver_) {
   LOG(LS_INFO) << "VideoReceiveStream: " << config_.ToString();
 
   RTC_DCHECK(process_thread_);
   RTC_DCHECK(congestion_controller_);
   RTC_DCHECK(call_stats_);
 
   RTC_DCHECK(!config_.decoders.empty());
   std::set<int> decoder_payload_types;
   for (const Decoder& decoder : config_.decoders) {

[tommi, 2016/10/07 19:45:40]

should this loop be moved into Start()?  As is, we don't seem to have symmetry
and two cycles of Start/Stop, would not do exactly the same thing.

[mflodman, 2016/10/09 09:22:26]

Yes, agree that should be done. Good catch Tommi!

     RTC_CHECK(decoder.decoder);
     RTC_CHECK(decoder_payload_types.find(decoder.payload_type) ==
               decoder_payload_types.end())
         << "Duplicate payload type (" << decoder.payload_type
         << ") for different decoders.";
     decoder_payload_types.insert(decoder.payload_type);
     video_receiver_.RegisterExternalDecoder(decoder.decoder,
                                             decoder.payload_type);
 
     VideoCodec codec = CreateDecoderVideoCodec(decoder);
     RTC_CHECK(rtp_stream_receiver_.SetReceiveCodec(codec));
     RTC_CHECK_EQ(VCM_OK, video_receiver_.RegisterReceiveCodec(
                              &codec, num_cpu_cores, false));
   }
 
   video_receiver_.SetRenderDelay(config.render_delay_ms);
 
   process_thread_->RegisterModule(&video_receiver_);
   process_thread_->RegisterModule(&rtp_stream_sync_);
 }
 
 VideoReceiveStream::~VideoReceiveStream() {
   LOG(LS_INFO) << "~VideoReceiveStream: " << config_.ToString();
   Stop();
 
   process_thread_->DeRegisterModule(&rtp_stream_sync_);
   process_thread_->DeRegisterModule(&video_receiver_);
 
-  // Deregister external decoders so they are no longer running during
-  // destruction. This effectively stops the VCM since the decoder thread is
-  // stopped, the VCM is deregistered and no asynchronous decoder threads are
-  // running.
-  for (const Decoder& decoder : config_.decoders)
-    video_receiver_.RegisterExternalDecoder(nullptr, decoder.payload_type);
-
   congestion_controller_->GetRemoteBitrateEstimator(UseSendSideBwe(config_))
       ->RemoveStream(rtp_stream_receiver_.GetRemoteSsrc());
 }
 
 void VideoReceiveStream::SignalNetworkState(NetworkState state) {
   rtp_stream_receiver_.SignalNetworkState(state);
 }
 
 
 bool VideoReceiveStream::DeliverRtcp(const uint8_t* packet, size_t length) {
@@ skipping 34 matching lines @@
   decode_thread_.SetPriority(rtc::kHighestPriority);
   rtp_stream_receiver_.StartReceive();
 }
 
 void VideoReceiveStream::Stop() {
   rtp_stream_receiver_.StopReceive();
   // TriggerDecoderShutdown will release any waiting decoder thread and make it
   // stop immediately, instead of waiting for a timeout. Needs to be called
   // before joining the decoder thread thread.
   video_receiver_.TriggerDecoderShutdown();
-  decode_thread_.Stop();
+  if (decode_thread_.IsRunning()) {
+    decode_thread_.Stop();
+    // Deregister external decoders so they are no longer running during
+    // destruction. This effectively stops the VCM since the decoder thread is
+    // stopped, the VCM is deregistered and no asynchronous decoder threads are
+    // running.
+    for (const Decoder& decoder : config_.decoders)
+      video_receiver_.RegisterExternalDecoder(nullptr, decoder.payload_type);
+  }
   call_stats_->DeregisterStatsObserver(video_stream_decoder_.get());
   video_stream_decoder_.reset();
   incoming_video_stream_.reset();
   transport_adapter_.Disable();
 }
 
 void VideoReceiveStream::SetSyncChannel(VoiceEngine* voice_engine,
                                         int audio_channel_id) {
   if (voice_engine && audio_channel_id != -1) {
     VoEVideoSync* voe_sync_interface = VoEVideoSync::GetInterface(voice_engine);
@@ skipping 88 matching lines @@
     RequestKeyFrame();
   }
 }
 
 void VideoReceiveStream::RequestKeyFrame() {
   rtp_stream_receiver_.RequestKeyFrame();
 }
 
 }  // namespace internal
 }  // namespace webrtc