commit 87d5a7494a04f452ff0fbf04841d4ecd55ca9a6c
author Taylor Brandstetter <deadbeef@webrtc.org> Tue Mar 06 17:42:25 2018
committer Commit Bot <commit-bot@chromium.org> Tue Mar 06 18:41:52 2018
tree 6632a34c7ff5ce4ce7ce7d6d7b42e7b0f5db1cea
parent 12eb85881c865a67be07d04856383fea22890d6a

Fix crash that occurs if GetStats is called from within OnStatsDelivered

This was resulting in the currently executing stats callback to be
invoked *again*, possibly ad infinitum, resulting in stack overflow.

Bug: webrtc:8973
Change-Id: Ib3bcc8e6bdd991728214fa242dda2010efc919a7
Reviewed-on: https://webrtc-review.googlesource.com/59464
Commit-Queue: Taylor Brandstetter <deadbeef@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#22313}

pc/rtcstatscollector.cc

diff --git a/pc/rtcstatscollector.cc b/pc/rtcstatscollector.cc
index 20cbc91..53a6158 100644
--- a/pc/rtcstatscollector.cc
+++ b/pc/rtcstatscollector.cc
@@ -780,11 +780,16 @@
   RTC_DCHECK(signaling_thread_->IsCurrent());
   RTC_DCHECK(!callbacks_.empty());
   RTC_DCHECK(cached_report_);
+
+  // Swap the list of callbacks, in case one of them recursively calls
+  // GetStatsReport again and modifies the callback list.
+  std::vector<rtc::scoped_refptr<RTCStatsCollectorCallback>> callbacks;
+  callbacks.swap(callbacks_);
+
   for (const rtc::scoped_refptr<RTCStatsCollectorCallback>& callback :
-       callbacks_) {
+       callbacks) {
     callback->OnStatsDelivered(cached_report_);
   }
-  callbacks_.clear();
 }
 
 void RTCStatsCollector::ProduceCertificateStats_n(

pc/rtcstatscollector_unittest.cc

diff --git a/pc/rtcstatscollector_unittest.cc b/pc/rtcstatscollector_unittest.cc
index f180471..5240e62 100644
--- a/pc/rtcstatscollector_unittest.cc
+++ b/pc/rtcstatscollector_unittest.cc
@@ -1910,6 +1910,37 @@
   EXPECT_EQ(1, track_stats.size());
 }
 
+// Used for test below, to test calling GetStatsReport during a callback.
+class ReentrantCallback : public RTCStatsCollectorCallback {
+ public:
+  explicit ReentrantCallback(RTCStatsCollectorWrapper* stats) : stats_(stats) {}
+
+  void OnStatsDelivered(
+      const rtc::scoped_refptr<const RTCStatsReport>& report) override {
+    stats_->GetStatsReport();
+    called_ = true;
+  }
+
+  bool called() const { return called_; }
+
+ private:
+  RTCStatsCollectorWrapper* stats_;
+  bool called_ = false;
+};
+
+// Test that nothing bad happens if a callback causes GetStatsReport to be
+// called again recursively. Regression test for crbug.com/webrtc/8973.
+TEST_F(RTCStatsCollectorTest, DoNotCrashOnReentrantInvocation) {
+  rtc::scoped_refptr<ReentrantCallback> callback1(
+      new rtc::RefCountedObject<ReentrantCallback>(stats_.get()));
+  rtc::scoped_refptr<ReentrantCallback> callback2(
+      new rtc::RefCountedObject<ReentrantCallback>(stats_.get()));
+  stats_->stats_collector()->GetStatsReport(callback1);
+  stats_->stats_collector()->GetStatsReport(callback2);
+  EXPECT_TRUE_WAIT(callback1->called(), kGetStatsReportTimeoutMs);
+  EXPECT_TRUE_WAIT(callback2->called(), kGetStatsReportTimeoutMs);
+}
+
 class RTCTestStats : public RTCStats {
  public:
   WEBRTC_RTCSTATS_DECL();