commit 499bc6c5d08999685d21152d8ac1423e68b5ce3f
author Yves Gerey <yvesg@webrtc.org> Wed Oct 10 18:29:07 2018 +0200
committer Commit Bot <commit-bot@chromium.org> Thu Oct 11 16:12:12 2018 +0000
tree e5c0f5fab7d1e776b19e420e9ab61c187033bb89
parent 53e22113fd7d0f43d6b1c0e97512316cce3c573d

Fix race conditions for ReofferDoesNotCallOnTrack test.

This CL extend critical sections to incorporate:
 * private_submodules_->echo_controller
 * config_

As a side benefit, it prevents weird interleaving where configuration
could have been changed in the middle of GetStatistics methods.

Bug: webrtc:9841
Change-Id: I0de5e756a684c2ff1be4effccf8c0f3d3175e3b9
Reviewed-on: https://webrtc-review.googlesource.com/c/105142
Reviewed-by: Gustaf Ullberg <gustaf@webrtc.org>
Commit-Queue: Yves Gerey <yvesg@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#25121}

modules/audio_processing/audio_processing_impl.cc

diff --git a/modules/audio_processing/audio_processing_impl.cc b/modules/audio_processing/audio_processing_impl.cc
index b9153fa..9015319 100644
--- a/modules/audio_processing/audio_processing_impl.cc
+++ b/modules/audio_processing/audio_processing_impl.cc
@@ -662,12 +662,12 @@
 }
 
 void AudioProcessingImpl::ApplyConfig(const AudioProcessing::Config& config) {
-  config_ = config;
-
   // Run in a single-threaded manner when applying the settings.
   rtc::CritScope cs_render(&crit_render_);
   rtc::CritScope cs_capture(&crit_capture_);
 
+  config_ = config;
+
   public_submodules_->echo_cancellation->Enable(
       config_.echo_canceller.enabled && !config_.echo_canceller.mobile_mode);
   public_submodules_->echo_control_mobile->Enable(
@@ -1642,8 +1642,8 @@
     const {
   AudioProcessingStatistics stats;
   EchoCancellationImpl::Metrics metrics;
+  rtc::CritScope cs_capture(&crit_capture_);
   if (private_submodules_->echo_controller) {
-    rtc::CritScope cs_capture(&crit_capture_);
     auto ec_metrics = private_submodules_->echo_controller->GetMetrics();
     float erl = static_cast<float>(ec_metrics.echo_return_loss);
     float erle = static_cast<float>(ec_metrics.echo_return_loss_enhancement);
@@ -1659,14 +1659,11 @@
         metrics.echo_return_loss_enhancement);
     stats.residual_echo_return_loss.Set(metrics.residual_echo_return_loss);
   }
-  {
-    rtc::CritScope cs_capture(&crit_capture_);
-    RTC_DCHECK(private_submodules_->echo_detector);
-    auto ed_metrics = private_submodules_->echo_detector->GetMetrics();
-    stats.residual_echo_likelihood = ed_metrics.echo_likelihood;
-    stats.residual_echo_likelihood_recent_max =
-        ed_metrics.echo_likelihood_recent_max;
-  }
+  RTC_DCHECK(private_submodules_->echo_detector);
+  auto ed_metrics = private_submodules_->echo_detector->GetMetrics();
+  stats.residual_echo_likelihood = ed_metrics.echo_likelihood;
+  stats.residual_echo_likelihood_recent_max =
+      ed_metrics.echo_likelihood_recent_max;
   public_submodules_->echo_cancellation->GetDelayMetrics(
       &stats.delay_median, &stats.delay_standard_deviation,
       &stats.fraction_poor_delays);
@@ -1678,8 +1675,8 @@
   AudioProcessingStats stats;
   if (has_remote_tracks) {
     EchoCancellationImpl::Metrics metrics;
+    rtc::CritScope cs_capture(&crit_capture_);
     if (private_submodules_->echo_controller) {
-      rtc::CritScope cs_capture(&crit_capture_);
       auto ec_metrics = private_submodules_->echo_controller->GetMetrics();
       stats.echo_return_loss = ec_metrics.echo_return_loss;
       stats.echo_return_loss_enhancement =
@@ -1701,7 +1698,6 @@
       }
     }
     if (config_.residual_echo_detector.enabled) {
-      rtc::CritScope cs_capture(&crit_capture_);
       RTC_DCHECK(private_submodules_->echo_detector);
       auto ed_metrics = private_submodules_->echo_detector->GetMetrics();
       stats.residual_echo_likelihood = ed_metrics.echo_likelihood;