commit dba22d31909298161318e00d43a80cdb0abc940f
author Tommi <tommi@webrtc.org> Thu Jun 01 14:08:52 2023
committer WebRTC LUCI CQ <webrtc-scoped@luci-project-accounts.iam.gserviceaccount.com> Thu Jun 01 16:29:46 2023
tree 6cfea55a9e106681549a31cc4f152618275c3696
parent 513ab0cb2c000b5613c6660259a31a7de48ef9d2

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 <hta@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40197}

pc/peer_connection.cc

diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc
index e0cef06..dd34f0f 100644
--- a/pc/peer_connection.cc
+++ b/pc/peer_connection.cc
@@ -731,9 +731,6 @@
   transport_controller_->SubscribeIceConnectionState(
       [this](cricket::IceConnectionState s) {
         RTC_DCHECK_RUN_ON(network_thread());
-        if (s == cricket::kIceConnectionConnected) {
-          ReportTransportStats();
-        }
         signaling_thread()->PostTask(
             SafeTask(signaling_thread_safety_.flag(), [this, s]() {
               RTC_DCHECK_RUN_ON(signaling_thread());
@@ -2397,6 +2394,20 @@
     case cricket::kIceConnectionConnected:
       RTC_LOG(LS_INFO) << "Changing to ICE connected state because "
                           "all transports are writable.";
+      {
+        std::vector<RtpTransceiverProxyRefPtr> transceivers;
+        if (ConfiguredForMedia()) {
+          transceivers = rtp_manager()->transceivers()->List();
+        }
+
+        network_thread()->PostTask(
+            SafeTask(network_thread_safety_,
+                     [this, transceivers = std::move(transceivers)] {
+                       RTC_DCHECK_RUN_ON(network_thread());
+                       ReportTransportStats(std::move(transceivers));
+                     }));
+      }
+
       SetIceConnectionState(PeerConnectionInterface::kIceConnectionConnected);
       NoteUsageEvent(UsageEvent::ICE_STATE_CONNECTED);
       break;
@@ -2740,20 +2751,18 @@
 }
 
 // Runs on network_thread().
-void PeerConnection::ReportTransportStats() {
+void PeerConnection::ReportTransportStats(
+    std::vector<RtpTransceiverProxyRefPtr> transceivers) {
   TRACE_EVENT0("webrtc", "PeerConnection::ReportTransportStats");
   rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls;
   std::map<std::string, std::set<cricket::MediaType>>
       media_types_by_transport_name;
-  if (ConfiguredForMedia()) {
-    for (const auto& transceiver :
-         rtp_manager()->transceivers()->UnsafeList()) {
-      if (transceiver->internal()->channel()) {
-        std::string transport_name(
-            transceiver->internal()->channel()->transport_name());
-        media_types_by_transport_name[transport_name].insert(
-            transceiver->media_type());
-      }
+  for (const auto& transceiver : transceivers) {
+    if (transceiver->internal()->channel()) {
+      std::string transport_name(
+          transceiver->internal()->channel()->transport_name());
+      media_types_by_transport_name[transport_name].insert(
+          transceiver->media_type());
     }
   }
 

pc/peer_connection.h

diff --git a/pc/peer_connection.h b/pc/peer_connection.h
index 26d2531..37f8df0 100644
--- a/pc/peer_connection.h
+++ b/pc/peer_connection.h
@@ -567,7 +567,8 @@
 
   // Invoked when TransportController connection completion is signaled.
   // Reports stats for all transports in use.
-  void ReportTransportStats() RTC_RUN_ON(network_thread());
+  void ReportTransportStats(std::vector<RtpTransceiverProxyRefPtr> transceivers)
+      RTC_RUN_ON(network_thread());
 
   // Gather the usage of IPv4/IPv6 as best connection.
   static void ReportBestConnectionState(const cricket::TransportStats& stats);

pc/peer_connection_integrationtest.cc

diff --git a/pc/peer_connection_integrationtest.cc b/pc/peer_connection_integrationtest.cc
index aa659b4..7fbfb7f 100644
--- a/pc/peer_connection_integrationtest.cc
+++ b/pc/peer_connection_integrationtest.cc
@@ -1731,6 +1731,10 @@
   EXPECT_EQ_WAIT(webrtc::PeerConnectionInterface::kIceConnectionConnected,
                  callee()->ice_connection_state(), kDefaultTimeout);
 
+  // Part of reporting the stats will occur on the network thread, so flush it
+  // before checking NumEvents.
+  SendTask(network_thread(), [] {});
+
   EXPECT_METRIC_EQ(1, webrtc::metrics::NumEvents(
                           "WebRTC.PeerConnection.CandidatePairType_UDP",
                           webrtc::kIceCandidatePairHostNameHostName));
@@ -1859,6 +1863,10 @@
   EXPECT_EQ_WAIT(webrtc::PeerConnectionInterface::kIceConnectionConnected,
                  callee()->ice_connection_state(), kDefaultTimeout);
 
+  // Part of reporting the stats will occur on the network thread, so flush it
+  // before checking NumEvents.
+  SendTask(network_thread(), [] {});
+
   // TODO(bugs.webrtc.org/9456): Fix it.
   const int num_best_ipv4 = webrtc::metrics::NumEvents(
       "WebRTC.PeerConnection.IPMetrics", webrtc::kBestConnections_IPv4);