Skip to content

Commit dba22d3

Browse files
TommiWebRTC LUCI CQ
authored andcommitted
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. 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-Commit-Position: refs/heads/main@{#40197}
1 parent 513ab0c commit dba22d3

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
@@ -731,9 +731,6 @@ JsepTransportController* PeerConnection::InitializeTransportController_n(
731731
transport_controller_->SubscribeIceConnectionState(
732732
[this](cricket::IceConnectionState s) {
733733
RTC_DCHECK_RUN_ON(network_thread());
734-
if (s == cricket::kIceConnectionConnected) {
735-
ReportTransportStats();
736-
}
737734
signaling_thread()->PostTask(
738735
SafeTask(signaling_thread_safety_.flag(), [this, s]() {
739736
RTC_DCHECK_RUN_ON(signaling_thread());
@@ -2397,6 +2394,20 @@ void PeerConnection::OnTransportControllerConnectionState(
23972394
case cricket::kIceConnectionConnected:
23982395
RTC_LOG(LS_INFO) << "Changing to ICE connected state because "
23992396
"all transports are writable.";
2397+
{
2398+
std::vector<RtpTransceiverProxyRefPtr> transceivers;
2399+
if (ConfiguredForMedia()) {
2400+
transceivers = rtp_manager()->transceivers()->List();
2401+
}
2402+
2403+
network_thread()->PostTask(
2404+
SafeTask(network_thread_safety_,
2405+
[this, transceivers = std::move(transceivers)] {
2406+
RTC_DCHECK_RUN_ON(network_thread());
2407+
ReportTransportStats(std::move(transceivers));
2408+
}));
2409+
}
2410+
24002411
SetIceConnectionState(PeerConnectionInterface::kIceConnectionConnected);
24012412
NoteUsageEvent(UsageEvent::ICE_STATE_CONNECTED);
24022413
break;
@@ -2740,20 +2751,18 @@ void PeerConnection::OnTransportControllerGatheringState(
27402751
}
27412752

27422753
// Runs on network_thread().
2743-
void PeerConnection::ReportTransportStats() {
2754+
void PeerConnection::ReportTransportStats(
2755+
std::vector<RtpTransceiverProxyRefPtr> transceivers) {
27442756
TRACE_EVENT0("webrtc", "PeerConnection::ReportTransportStats");
27452757
rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls;
27462758
std::map<std::string, std::set<cricket::MediaType>>
27472759
media_types_by_transport_name;
2748-
if (ConfiguredForMedia()) {
2749-
for (const auto& transceiver :
2750-
rtp_manager()->transceivers()->UnsafeList()) {
2751-
if (transceiver->internal()->channel()) {
2752-
std::string transport_name(
2753-
transceiver->internal()->channel()->transport_name());
2754-
media_types_by_transport_name[transport_name].insert(
2755-
transceiver->media_type());
2756-
}
2760+
for (const auto& transceiver : transceivers) {
2761+
if (transceiver->internal()->channel()) {
2762+
std::string transport_name(
2763+
transceiver->internal()->channel()->transport_name());
2764+
media_types_by_transport_name[transport_name].insert(
2765+
transceiver->media_type());
27572766
}
27582767
}
27592768

pc/peer_connection.h

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

568568
// Invoked when TransportController connection completion is signaled.
569569
// Reports stats for all transports in use.
570-
void ReportTransportStats() RTC_RUN_ON(network_thread());
570+
void ReportTransportStats(std::vector<RtpTransceiverProxyRefPtr> transceivers)
571+
RTC_RUN_ON(network_thread());
571572

572573
// Gather the usage of IPv4/IPv6 as best connection.
573574
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
@@ -1731,6 +1731,10 @@ TEST_P(PeerConnectionIntegrationTest,
17311731
EXPECT_EQ_WAIT(webrtc::PeerConnectionInterface::kIceConnectionConnected,
17321732
callee()->ice_connection_state(), kDefaultTimeout);
17331733

1734+
// Part of reporting the stats will occur on the network thread, so flush it
1735+
// before checking NumEvents.
1736+
SendTask(network_thread(), [] {});
1737+
17341738
EXPECT_METRIC_EQ(1, webrtc::metrics::NumEvents(
17351739
"WebRTC.PeerConnection.CandidatePairType_UDP",
17361740
webrtc::kIceCandidatePairHostNameHostName));
@@ -1859,6 +1863,10 @@ TEST_P(PeerConnectionIntegrationIceStatesTest, MAYBE_VerifyBestConnection) {
18591863
EXPECT_EQ_WAIT(webrtc::PeerConnectionInterface::kIceConnectionConnected,
18601864
callee()->ice_connection_state(), kDefaultTimeout);
18611865

1866+
// Part of reporting the stats will occur on the network thread, so flush it
1867+
// before checking NumEvents.
1868+
SendTask(network_thread(), [] {});
1869+
18621870
// TODO(bugs.webrtc.org/9456): Fix it.
18631871
const int num_best_ipv4 = webrtc::metrics::NumEvents(
18641872
"WebRTC.PeerConnection.IPMetrics", webrtc::kBestConnections_IPv4);

0 commit comments

Comments
 (0)