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}
This commit is contained in:
committed by
Commit Bot
parent
12eb85881c
commit
87d5a7494a
@ -780,11 +780,16 @@ void RTCStatsCollector::DeliverCachedReport() {
|
||||
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(
|
||||
|
||||
@ -1910,6 +1910,37 @@ TEST_F(RTCStatsCollectorTest, DoNotCrashOnSsrcChange) {
|
||||
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();
|
||||
|
||||
Reference in New Issue
Block a user