From 4515fa0bed80d4a8e31836a73c14aade02bf68ba Mon Sep 17 00:00:00 2001
From: henrika <henrika@webrtc.org>
Date: Wed, 3 May 2017 08:30:15 -0700
Subject: [PATCH] Resolves race between Channel::ProcessAndEncodeAudio() and
 Channel::StopSend()

BUG=webrtc:7540

Review-Url: https://codereview.webrtc.org/2861583005
Cr-Commit-Position: refs/heads/master@{#17999}
---
 webrtc/voice_engine/channel.cc | 27 +++++++++++++++++++++++----
 webrtc/voice_engine/channel.h  |  4 ++++
 2 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/webrtc/voice_engine/channel.cc b/webrtc/voice_engine/channel.cc
index f56d1d822c..6f01725392 100644
--- a/webrtc/voice_engine/channel.cc
+++ b/webrtc/voice_engine/channel.cc
@@ -1217,7 +1217,11 @@ int32_t Channel::StartSend() {
     return 0;
   }
   channel_state_.SetSending(true);
-
+  {
+    // It is now OK to start posting tasks to the encoder task queue.
+    rtc::CritScope cs(&encoder_queue_lock_);
+    encoder_queue_is_active_ = true;
+  }
   // Resume the previous sequence number which was reset by StopSend(). This
   // needs to be done before |sending| is set to true on the RTP/RTCP module.
   if (send_sequence_number_) {
@@ -1252,8 +1256,15 @@ void Channel::StopSend() {
   // exists and it is therfore guaranteed that the task queue will never try
   // to acccess and invalid channel object.
   RTC_DCHECK(encoder_queue_);
+
   rtc::Event flush(false, false);
-  encoder_queue_->PostTask([&flush]() { flush.Set(); });
+  {
+    // Clear |encoder_queue_is_active_| under lock to prevent any other tasks
+    // than this final "flush task" to be posted on the queue.
+    rtc::CritScope cs(&encoder_queue_lock_);
+    encoder_queue_is_active_ = false;
+    encoder_queue_->PostTask([&flush]() { flush.Set(); });
+  }
   flush.Wait(rtc::Event::kForever);
 
   // Store the sequence number to be able to pick up the same sequence for
@@ -2724,7 +2735,11 @@ int Channel::ResendPackets(const uint16_t* sequence_numbers, int length) {
 }
 
 void Channel::ProcessAndEncodeAudio(const AudioFrame& audio_input) {
-  RTC_DCHECK(channel_state_.Get().sending);
+  // Avoid posting any new tasks if sending was already stopped in StopSend().
+  rtc::CritScope cs(&encoder_queue_lock_);
+  if (!encoder_queue_is_active_) {
+    return;
+  }
   std::unique_ptr<AudioFrame> audio_frame(new AudioFrame());
   // TODO(henrika): try to avoid copying by moving ownership of audio frame
   // either into pool of frames or into the task itself.
@@ -2738,7 +2753,11 @@ void Channel::ProcessAndEncodeAudio(const int16_t* audio_data,
                                     int sample_rate,
                                     size_t number_of_frames,
                                     size_t number_of_channels) {
-  RTC_DCHECK(channel_state_.Get().sending);
+  // Avoid posting as new task if sending was already stopped in StopSend().
+  rtc::CritScope cs(&encoder_queue_lock_);
+  if (!encoder_queue_is_active_) {
+    return;
+  }
   CodecInst codec;
   GetSendCodec(codec);
   std::unique_ptr<AudioFrame> audio_frame(new AudioFrame());
diff --git a/webrtc/voice_engine/channel.h b/webrtc/voice_engine/channel.h
index d5eb5d954c..b7b59a447c 100644
--- a/webrtc/voice_engine/channel.h
+++ b/webrtc/voice_engine/channel.h
@@ -546,6 +546,10 @@ class Channel
 
   const bool use_twcc_plr_for_ana_;
 
+  rtc::CriticalSection encoder_queue_lock_;
+
+  bool encoder_queue_is_active_ GUARDED_BY(encoder_queue_lock_) = false;
+
   rtc::TaskQueue* encoder_queue_ = nullptr;
 };