[Stats] Add more rtc::Thread::ScopedDisallowBlockingCalls to getStats().
This ensures with DCHECK-crashes that we don't accidentally do more
blocking invokes than we think.
Remaining blocking invokes FYI:
- PrepareTransceiverStatsInfos_s_w() does 1 blocking invoke (regardless
of the number of transceivers or channels) to the worker thread. This
is because VoiceMediaChannel, VideoMediaChannel and GetParameters()
execute on the worker thread, and the result of these operations are
needed on the signalling thread.
- pc_->GetCallStats() does 1 blocking invoke to the worker thread.
These two blocking invokes can be merged, reducing the total number of
blocking invokes from 2 to 1, but this CL does not attempt to do that.
I filed https://crbug.com/webrtc/11767 for that.
Bug: webrtc:11716
Change-Id: Iebc2ab350d253fd037211cdd283825b4e5b2d446
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/178867
Reviewed-by: Evan Shrubsole <eshr@google.com>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31670}
diff --git a/pc/rtc_stats_collector.cc b/pc/rtc_stats_collector.cc
index e1527d5..53b970c 100644
--- a/pc/rtc_stats_collector.cc
+++ b/pc/rtc_stats_collector.cc
@@ -1040,7 +1040,7 @@
// Prepare |transceiver_stats_infos_| for use in
// |ProducePartialResultsOnNetworkThread| and
// |ProducePartialResultsOnSignalingThread|.
- transceiver_stats_infos_ = PrepareTransceiverStatsInfos_s();
+ transceiver_stats_infos_ = PrepareTransceiverStatsInfos_s_w();
// Prepare |transport_names_| for use in
// |ProducePartialResultsOnNetworkThread|.
transport_names_ = PrepareTransportNames_s();
@@ -1049,6 +1049,10 @@
// thread.
// TODO(holmer): To avoid the hop we could move BWE and BWE stats to the
// network thread, where it more naturally belongs.
+ // TODO(https://crbug.com/webrtc/11767): In the meantime we can piggyback on
+ // the blocking-invoke that is already performed in
+ // PrepareTransceiverStatsInfos_s_w() so that we can call GetCallStats()
+ // without additional blocking-invokes.
call_stats_ = pc_->GetCallStats();
// Don't touch |network_report_| on the signaling thread until
@@ -1078,6 +1082,8 @@
void RTCStatsCollector::ProducePartialResultsOnSignalingThread(
int64_t timestamp_us) {
RTC_DCHECK(signaling_thread_->IsCurrent());
+ rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls;
+
partial_report_ = RTCStatsReport::Create(timestamp_us);
ProducePartialResultsOnSignalingThreadImpl(timestamp_us,
@@ -1095,6 +1101,8 @@
int64_t timestamp_us,
RTCStatsReport* partial_report) {
RTC_DCHECK(signaling_thread_->IsCurrent());
+ rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls;
+
ProduceDataChannelStats_s(timestamp_us, partial_report);
ProduceMediaStreamStats_s(timestamp_us, partial_report);
ProduceMediaStreamTrackStats_s(timestamp_us, partial_report);
@@ -1105,6 +1113,8 @@
void RTCStatsCollector::ProducePartialResultsOnNetworkThread(
int64_t timestamp_us) {
RTC_DCHECK(network_thread_->IsCurrent());
+ rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls;
+
// Touching |network_report_| on this thread is safe by this method because
// |network_report_event_| is reset before this method is invoked.
network_report_ = RTCStatsReport::Create(timestamp_us);
@@ -1132,6 +1142,8 @@
const std::map<std::string, CertificateStatsPair>& transport_cert_stats,
RTCStatsReport* partial_report) {
RTC_DCHECK(network_thread_->IsCurrent());
+ rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls;
+
ProduceCertificateStats_n(timestamp_us, transport_cert_stats, partial_report);
ProduceCodecStats_n(timestamp_us, transceiver_stats_infos_, partial_report);
ProduceIceCandidateAndPairStats_n(timestamp_us, transport_stats_by_name,
@@ -1218,6 +1230,8 @@
const std::map<std::string, CertificateStatsPair>& transport_cert_stats,
RTCStatsReport* report) const {
RTC_DCHECK(network_thread_->IsCurrent());
+ rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls;
+
for (const auto& transport_cert_stats_pair : transport_cert_stats) {
if (transport_cert_stats_pair.second.local) {
ProduceCertificateStatsFromSSLCertificateStats(
@@ -1235,6 +1249,8 @@
const std::vector<RtpTransceiverStatsInfo>& transceiver_stats_infos,
RTCStatsReport* report) const {
RTC_DCHECK(network_thread_->IsCurrent());
+ rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls;
+
for (const auto& stats : transceiver_stats_infos) {
if (!stats.mid) {
continue;
@@ -1276,6 +1292,8 @@
int64_t timestamp_us,
RTCStatsReport* report) const {
RTC_DCHECK_RUN_ON(signaling_thread_);
+ rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls;
+
std::vector<DataChannel::Stats> data_stats = pc_->GetDataChannelStats();
for (const auto& stats : data_stats) {
std::unique_ptr<RTCDataChannelStats> data_channel_stats(
@@ -1301,6 +1319,8 @@
const Call::Stats& call_stats,
RTCStatsReport* report) const {
RTC_DCHECK(network_thread_->IsCurrent());
+ rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls;
+
for (const auto& entry : transport_stats_by_name) {
const std::string& transport_name = entry.first;
const cricket::TransportStats& transport_stats = entry.second;
@@ -1381,6 +1401,7 @@
int64_t timestamp_us,
RTCStatsReport* report) const {
RTC_DCHECK(signaling_thread_->IsCurrent());
+ rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls;
std::map<std::string, std::vector<std::string>> track_ids;
@@ -1417,6 +1438,8 @@
int64_t timestamp_us,
RTCStatsReport* report) const {
RTC_DCHECK(signaling_thread_->IsCurrent());
+ rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls;
+
for (const RtpTransceiverStatsInfo& stats : transceiver_stats_infos_) {
std::vector<rtc::scoped_refptr<RtpSenderInternal>> senders;
for (const auto& sender : stats.transceiver->senders()) {
@@ -1438,6 +1461,8 @@
int64_t timestamp_us,
RTCStatsReport* report) const {
RTC_DCHECK(signaling_thread_->IsCurrent());
+ rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls;
+
for (const RtpTransceiverStatsInfo& transceiver_stats_info :
transceiver_stats_infos_) {
const auto& track_media_info_map =
@@ -1519,6 +1544,8 @@
int64_t timestamp_us,
RTCStatsReport* report) const {
RTC_DCHECK(signaling_thread_->IsCurrent());
+ rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls;
+
std::unique_ptr<RTCPeerConnectionStats> stats(
new RTCPeerConnectionStats("RTCPeerConnection", timestamp_us));
stats->data_channels_opened = internal_record_.data_channels_opened;
@@ -1531,6 +1558,7 @@
const std::vector<RtpTransceiverStatsInfo>& transceiver_stats_infos,
RTCStatsReport* report) const {
RTC_DCHECK(network_thread_->IsCurrent());
+ rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls;
for (const RtpTransceiverStatsInfo& stats : transceiver_stats_infos) {
if (stats.media_type == cricket::MEDIA_TYPE_AUDIO) {
@@ -1547,6 +1575,9 @@
int64_t timestamp_us,
const RtpTransceiverStatsInfo& stats,
RTCStatsReport* report) const {
+ RTC_DCHECK(network_thread_->IsCurrent());
+ rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls;
+
if (!stats.mid || !stats.transport_name) {
return;
}
@@ -1625,6 +1656,9 @@
int64_t timestamp_us,
const RtpTransceiverStatsInfo& stats,
RTCStatsReport* report) const {
+ RTC_DCHECK(network_thread_->IsCurrent());
+ rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls;
+
if (!stats.mid || !stats.transport_name) {
return;
}
@@ -1705,6 +1739,8 @@
const std::map<std::string, CertificateStatsPair>& transport_cert_stats,
RTCStatsReport* report) const {
RTC_DCHECK(network_thread_->IsCurrent());
+ rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls;
+
for (const auto& entry : transport_stats_by_name) {
const std::string& transport_name = entry.first;
const cricket::TransportStats& transport_stats = entry.second;
@@ -1796,6 +1832,8 @@
const std::map<std::string, cricket::TransportStats>&
transport_stats_by_name) const {
RTC_DCHECK(network_thread_->IsCurrent());
+ rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls;
+
std::map<std::string, CertificateStatsPair> transport_cert_stats;
for (const auto& entry : transport_stats_by_name) {
const std::string& transport_name = entry.first;
@@ -1820,9 +1858,10 @@
}
std::vector<RTCStatsCollector::RtpTransceiverStatsInfo>
-RTCStatsCollector::PrepareTransceiverStatsInfos_s() const {
- std::vector<RtpTransceiverStatsInfo> transceiver_stats_infos;
+RTCStatsCollector::PrepareTransceiverStatsInfos_s_w() const {
+ RTC_DCHECK(signaling_thread_->IsCurrent());
+ std::vector<RtpTransceiverStatsInfo> transceiver_stats_infos;
// These are used to invoke GetStats for all the media channels together in
// one worker thread hop.
std::map<cricket::VoiceMediaChannel*,
@@ -1832,39 +1871,43 @@
std::unique_ptr<cricket::VideoMediaInfo>>
video_stats;
- for (const auto& transceiver : pc_->GetTransceiversInternal()) {
- cricket::MediaType media_type = transceiver->media_type();
+ {
+ rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls;
- // Prepare stats entry. The TrackMediaInfoMap will be filled in after the
- // stats have been fetched on the worker thread.
- transceiver_stats_infos.emplace_back();
- RtpTransceiverStatsInfo& stats = transceiver_stats_infos.back();
- stats.transceiver = transceiver->internal();
- stats.media_type = media_type;
+ for (const auto& transceiver : pc_->GetTransceiversInternal()) {
+ cricket::MediaType media_type = transceiver->media_type();
- cricket::ChannelInterface* channel = transceiver->internal()->channel();
- if (!channel) {
- // The remaining fields require a BaseChannel.
- continue;
- }
+ // Prepare stats entry. The TrackMediaInfoMap will be filled in after the
+ // stats have been fetched on the worker thread.
+ transceiver_stats_infos.emplace_back();
+ RtpTransceiverStatsInfo& stats = transceiver_stats_infos.back();
+ stats.transceiver = transceiver->internal();
+ stats.media_type = media_type;
- stats.mid = channel->content_name();
- stats.transport_name = channel->transport_name();
+ cricket::ChannelInterface* channel = transceiver->internal()->channel();
+ if (!channel) {
+ // The remaining fields require a BaseChannel.
+ continue;
+ }
- if (media_type == cricket::MEDIA_TYPE_AUDIO) {
- auto* voice_channel = static_cast<cricket::VoiceChannel*>(channel);
- RTC_DCHECK(voice_stats.find(voice_channel->media_channel()) ==
- voice_stats.end());
- voice_stats[voice_channel->media_channel()] =
- std::make_unique<cricket::VoiceMediaInfo>();
- } else if (media_type == cricket::MEDIA_TYPE_VIDEO) {
- auto* video_channel = static_cast<cricket::VideoChannel*>(channel);
- RTC_DCHECK(video_stats.find(video_channel->media_channel()) ==
- video_stats.end());
- video_stats[video_channel->media_channel()] =
- std::make_unique<cricket::VideoMediaInfo>();
- } else {
- RTC_NOTREACHED();
+ stats.mid = channel->content_name();
+ stats.transport_name = channel->transport_name();
+
+ if (media_type == cricket::MEDIA_TYPE_AUDIO) {
+ auto* voice_channel = static_cast<cricket::VoiceChannel*>(channel);
+ RTC_DCHECK(voice_stats.find(voice_channel->media_channel()) ==
+ voice_stats.end());
+ voice_stats[voice_channel->media_channel()] =
+ std::make_unique<cricket::VoiceMediaInfo>();
+ } else if (media_type == cricket::MEDIA_TYPE_VIDEO) {
+ auto* video_channel = static_cast<cricket::VideoChannel*>(channel);
+ RTC_DCHECK(video_stats.find(video_channel->media_channel()) ==
+ video_stats.end());
+ video_stats[video_channel->media_channel()] =
+ std::make_unique<cricket::VideoMediaInfo>();
+ } else {
+ RTC_NOTREACHED();
+ }
}
}
@@ -1924,6 +1967,9 @@
}
std::set<std::string> RTCStatsCollector::PrepareTransportNames_s() const {
+ RTC_DCHECK(signaling_thread_->IsCurrent());
+ rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls;
+
std::set<std::string> transport_names;
for (const auto& transceiver : pc_->GetTransceiversInternal()) {
if (transceiver->internal()->channel()) {
