From 0d348d69e6c6cf58e07e12b43a06566de580bdfc Mon Sep 17 00:00:00 2001 From: sprang Date: Fri, 7 Oct 2016 08:28:39 -0700 Subject: [PATCH] Avoid race in VideoReceiveStream shutdown The decoder implementation may have its own thread for asynchrouns callbacks. We need to stop it (by releasing the decoder) when stopping the decoder thread, othweise it may call incoming_video_stream_ after it has been destroyed. BUG=webrtc:6501 Review-Url: https://codereview.webrtc.org/2402663003 Cr-Commit-Position: refs/heads/master@{#14577} --- webrtc/video/video_receive_stream.cc | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/webrtc/video/video_receive_stream.cc b/webrtc/video/video_receive_stream.cc index 2b4a3c2b7b..7d77751db3 100644 --- a/webrtc/video/video_receive_stream.cc +++ b/webrtc/video/video_receive_stream.cc @@ -246,13 +246,6 @@ VideoReceiveStream::~VideoReceiveStream() { 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()); } @@ -307,7 +300,15 @@ void VideoReceiveStream::Stop() { // 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();