admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Deliver cached stats reports asynchronously.

Browse Source 
This has the following benefits:
* Stats reports are always delivered asynchronously. This means the API
  client doesn't need to worry about *possibly* getting a synchronous
  callback depending on when the last report was generated.
* Stats callbacks will always be invoked in the same order that the
  GetStats calls were made, even in cases where a callback recursively
  calls GetStats again.

Bug: webrtc:8973
Change-Id: I94ca4b5dc5c21a8f2df42adfcddf357f40a32025
Reviewed-on: https://webrtc-review.googlesource.com/60473
Commit-Queue: Taylor Brandstetter <deadbeef@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#22348}
This commit is contained in:
Taylor Brandstetter 2018-03-08 09:53:47 -08:00 committed by Commit Bot
parent 6328d7cbbc
commit 25e022fd5c
3 changed files with 32 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

34
pc/rtcstatscollector.cc
View File

@ -648,8 +648,16 @@ void RTCStatsCollector::GetStatsReport(
int64_t cache_now_us = rtc::TimeMicros();
if (cached_report_ &&
cache_now_us - cache_timestamp_us_ <= cache_lifetime_us_) {
// We have a fresh cached report to deliver.
DeliverCachedReport();
// We have a fresh cached report to deliver. Deliver asynchronously, since
// the caller may not be expecting a synchronous callback, and it avoids
// reentrancy problems.
std::vector<rtc::scoped_refptr<RTCStatsCollectorCallback>> callbacks;
callbacks.swap(callbacks_);
invoker_.AsyncInvoke<void>(
RTC_FROM_HERE, signaling_thread_,
rtc::Bind(&RTCStatsCollector::DeliverCachedReport, this, cached_report_,
std::move(callbacks)));
callbacks_.clear();
} else if (!num_pending_partial_reports_) {
// Only start gathering stats if we're not already gathering stats. In the
// case of already gathering stats, |callback_| will be invoked when there
@ -772,23 +780,25 @@ void RTCStatsCollector::AddPartialResults_s(
// select the "webrtc_stats" category when recording traces.
TRACE_EVENT_INSTANT1("webrtc_stats", "webrtc_stats", "report",
cached_report_->ToJson());
DeliverCachedReport();
// 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_);
DeliverCachedReport(cached_report_, std::move(callbacks));
}
}
void RTCStatsCollector::DeliverCachedReport() {
void RTCStatsCollector::DeliverCachedReport(
rtc::scoped_refptr<const RTCStatsReport> cached_report,
std::vector<rtc::scoped_refptr<RTCStatsCollectorCallback>> callbacks) {
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_);
RTC_DCHECK(!callbacks.empty());
RTC_DCHECK(cached_report);
for (const rtc::scoped_refptr<RTCStatsCollectorCallback>& callback :
callbacks) {
callback->OnStatsDelivered(cached_report_);
callback->OnStatsDelivered(cached_report);
}
}

4
pc/rtcstatscollector.h
View File

@ -94,7 +94,9 @@ class RTCStatsCollector : public virtual rtc::RefCountInterface,
};
void AddPartialResults_s(rtc::scoped_refptr<RTCStatsReport> partial_report);
void DeliverCachedReport();
void DeliverCachedReport(
rtc::scoped_refptr<const RTCStatsReport> cached_report,
std::vector<rtc::scoped_refptr<RTCStatsCollectorCallback>> callbacks);
// Produces |RTCCertificateStats|.
void ProduceCertificateStats_n(

14
pc/rtcstatscollector_unittest.cc
View File

@ -1911,9 +1911,9 @@ TEST_F(RTCStatsCollectorTest, DoNotCrashOnSsrcChange) {
}
// Used for test below, to test calling GetStatsReport during a callback.
class ReentrantCallback : public RTCStatsCollectorCallback {
class RecursiveCallback : public RTCStatsCollectorCallback {
public:
explicit ReentrantCallback(RTCStatsCollectorWrapper* stats) : stats_(stats) {}
explicit RecursiveCallback(RTCStatsCollectorWrapper* stats) : stats_(stats) {}
void OnStatsDelivered(
const rtc::scoped_refptr<const RTCStatsReport>& report) override {
@ -1930,11 +1930,11 @@ class ReentrantCallback : public RTCStatsCollectorCallback {
// 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()));
TEST_F(RTCStatsCollectorTest, DoNotCrashWhenGetStatsCalledDuringCallback) {
rtc::scoped_refptr<RecursiveCallback> callback1(
new rtc::RefCountedObject<RecursiveCallback>(stats_.get()));
rtc::scoped_refptr<RecursiveCallback> callback2(
new rtc::RefCountedObject<RecursiveCallback>(stats_.get()));
stats_->stats_collector()->GetStatsReport(callback1);
stats_->stats_collector()->GetStatsReport(callback2);
EXPECT_TRUE_WAIT(callback1->called(), kGetStatsReportTimeoutMs);