From 25cfeb9ef13fd3c3cde37819805ec54e34dc2310 Mon Sep 17 00:00:00 2001 From: Steve Anton Date: Thu, 26 Apr 2018 11:44:00 -0700 Subject: [PATCH] Fix possible race between the stats collector and transport controller MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, it would be possible for the RTCStatsCollector to start an async network task to gather stats that would be run after the PeerConnection was closed when the transport controller was set to null. Bug: chromium:829238 Change-Id: I22fb4ce603caf2614814780b95b62127cee28284 Reviewed-on: https://webrtc-review.googlesource.com/72525 Reviewed-by: Henrik Boström Reviewed-by: Taylor Brandstetter Cr-Commit-Position: refs/heads/master@{#23046} --- pc/peerconnection.cc | 9 +++++++++ pc/rtcstats_integrationtest.cc | 15 ++++++++++++++- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/pc/peerconnection.cc b/pc/peerconnection.cc index fa2ba7c91e..b7fe2e3ad3 100644 --- a/pc/peerconnection.cc +++ b/pc/peerconnection.cc @@ -3210,6 +3210,15 @@ void PeerConnection::Close() { for (auto transceiver : transceivers_) { transceiver->Stop(); } + + // Ensure that all asynchronous stats requests are completed before destroying + // the transport controller below. + if (stats_collector_) { + stats_collector_->WaitForPendingRequest(); + } + + // Don't destroy BaseChannels until after stats has been cleaned up so that + // the last stats request can still read from the channels. DestroyAllChannels(); // The event log is used in the transport controller, which must be outlived diff --git a/pc/rtcstats_integrationtest.cc b/pc/rtcstats_integrationtest.cc index cdc46d7d81..627a05bb9a 100644 --- a/pc/rtcstats_integrationtest.cc +++ b/pc/rtcstats_integrationtest.cc @@ -826,7 +826,20 @@ TEST_F(RTCStatsIntegrationTest, GetsStatsWhileDestroyingPeerConnections) { caller_ = nullptr; // Any pending stats requests should have completed in the act of destroying // the peer connection. - EXPECT_TRUE(stats_obtainer->report()); + ASSERT_TRUE(stats_obtainer->report()); + EXPECT_EQ(stats_obtainer->report()->ToJson(), + RTCStatsReportTraceListener::last_trace()); +} + +TEST_F(RTCStatsIntegrationTest, GetsStatsWhileClosingPeerConnection) { + StartCall(); + + rtc::scoped_refptr stats_obtainer = + RTCStatsObtainer::Create(); + caller_->pc()->GetStats(stats_obtainer); + caller_->pc()->Close(); + + ASSERT_TRUE(stats_obtainer->report()); EXPECT_EQ(stats_obtainer->report()->ToJson(), RTCStatsReportTraceListener::last_trace()); }