Skip to content

Commit e46e37b

Browse files
TommiWebRTC LUCI CQ
authored andcommitted
[M114] Move transceiver iteration loop over to the signaling thread.
This is required for ReportTransportStats since iterating over the transceiver list from the network thread is not safe. (cherry picked from commit dba22d3) No-Try: true Bug: chromium:1446274, webrtc:12692 Change-Id: I7c514df9f029112c4b1da85826af91217850fb26 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/307340 Reviewed-by: Harald Alvestrand <[email protected]> Commit-Queue: Tomas Gunnarsson <[email protected]> Cr-Original-Commit-Position: refs/heads/main@{#40197} Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/308001 Reviewed-by: Mirko Bonadei <[email protected]> Cr-Commit-Position: refs/branch-heads/5735@{#3} Cr-Branched-From: df7df19-refs/heads/main@{#39949}
1 parent ecab2a4 commit e46e37b

File tree

3 files changed

+32
-14
lines changed

3 files changed

+32
-14
lines changed

pc/peer_connection.cc

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -735,9 +735,6 @@ JsepTransportController* PeerConnection::InitializeTransportController_n(
735735
transport_controller_->SubscribeIceConnectionState(
736736
[this](cricket::IceConnectionState s) {
737737
RTC_DCHECK_RUN_ON(network_thread());
738-
if (s == cricket::kIceConnectionConnected) {
739-
ReportTransportStats();
740-
}
741738
signaling_thread()->PostTask(
742739
SafeTask(signaling_thread_safety_.flag(), [this, s]() {
743740
RTC_DCHECK_RUN_ON(signaling_thread());
@@ -2401,6 +2398,20 @@ void PeerConnection::OnTransportControllerConnectionState(
24012398
case cricket::kIceConnectionConnected:
24022399
RTC_LOG(LS_INFO) << "Changing to ICE connected state because "
24032400
"all transports are writable.";
2401+
{
2402+
std::vector<RtpTransceiverProxyRefPtr> transceivers;
2403+
if (ConfiguredForMedia()) {
2404+
transceivers = rtp_manager()->transceivers()->List();
2405+
}
2406+
2407+
network_thread()->PostTask(
2408+
SafeTask(network_thread_safety_,
2409+
[this, transceivers = std::move(transceivers)] {
2410+
RTC_DCHECK_RUN_ON(network_thread());
2411+
ReportTransportStats(std::move(transceivers));
2412+
}));
2413+
}
2414+
24042415
SetIceConnectionState(PeerConnectionInterface::kIceConnectionConnected);
24052416
NoteUsageEvent(UsageEvent::ICE_STATE_CONNECTED);
24062417
break;
@@ -2743,20 +2754,18 @@ void PeerConnection::OnTransportControllerGatheringState(
27432754
}
27442755

27452756
// Runs on network_thread().
2746-
void PeerConnection::ReportTransportStats() {
2757+
void PeerConnection::ReportTransportStats(
2758+
std::vector<RtpTransceiverProxyRefPtr> transceivers) {
27472759
TRACE_EVENT0("webrtc", "PeerConnection::ReportTransportStats");
27482760
rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls;
27492761
std::map<std::string, std::set<cricket::MediaType>>
27502762
media_types_by_transport_name;
2751-
if (ConfiguredForMedia()) {
2752-
for (const auto& transceiver :
2753-
rtp_manager()->transceivers()->UnsafeList()) {
2754-
if (transceiver->internal()->channel()) {
2755-
std::string transport_name(
2756-
transceiver->internal()->channel()->transport_name());
2757-
media_types_by_transport_name[transport_name].insert(
2758-
transceiver->media_type());
2759-
}
2763+
for (const auto& transceiver : transceivers) {
2764+
if (transceiver->internal()->channel()) {
2765+
std::string transport_name(
2766+
transceiver->internal()->channel()->transport_name());
2767+
media_types_by_transport_name[transport_name].insert(
2768+
transceiver->media_type());
27602769
}
27612770
}
27622771

pc/peer_connection.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -569,7 +569,8 @@ class PeerConnection : public PeerConnectionInternal,
569569

570570
// Invoked when TransportController connection completion is signaled.
571571
// Reports stats for all transports in use.
572-
void ReportTransportStats() RTC_RUN_ON(network_thread());
572+
void ReportTransportStats(std::vector<RtpTransceiverProxyRefPtr> transceivers)
573+
RTC_RUN_ON(network_thread());
573574

574575
// Gather the usage of IPv4/IPv6 as best connection.
575576
static void ReportBestConnectionState(const cricket::TransportStats& stats);

pc/peer_connection_integrationtest.cc

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1831,6 +1831,10 @@ TEST_P(PeerConnectionIntegrationTest,
18311831
EXPECT_EQ_WAIT(webrtc::PeerConnectionInterface::kIceConnectionConnected,
18321832
callee()->ice_connection_state(), kDefaultTimeout);
18331833

1834+
// Part of reporting the stats will occur on the network thread, so flush it
1835+
// before checking NumEvents.
1836+
SendTask(network_thread(), [] {});
1837+
18341838
EXPECT_METRIC_EQ(1, webrtc::metrics::NumEvents(
18351839
"WebRTC.PeerConnection.CandidatePairType_UDP",
18361840
webrtc::kIceCandidatePairHostNameHostName));
@@ -1959,6 +1963,10 @@ TEST_P(PeerConnectionIntegrationIceStatesTest, MAYBE_VerifyBestConnection) {
19591963
EXPECT_EQ_WAIT(webrtc::PeerConnectionInterface::kIceConnectionConnected,
19601964
callee()->ice_connection_state(), kDefaultTimeout);
19611965

1966+
// Part of reporting the stats will occur on the network thread, so flush it
1967+
// before checking NumEvents.
1968+
SendTask(network_thread(), [] {});
1969+
19621970
// TODO(bugs.webrtc.org/9456): Fix it.
19631971
const int num_best_ipv4 = webrtc::metrics::NumEvents(
19641972
"WebRTC.PeerConnection.IPMetrics", webrtc::kBestConnections_IPv4);

0 commit comments

Comments
 (0)