From 82df4ce4226de25e1682351d7ba2e9bf9a198790 Mon Sep 17 00:00:00 2001
From: Tommi <[email protected]>
Date: Fri, 25 Sep 2026 11:12:56 +0200
Subject: [PATCH] Ensure stopped transceivers do not hold a channel

Make sure that a transceiver that has been stopped does not get a
channel during negotiation, and that any channel is cleared before a
stopped transceiver is removed from the transceiver list.

Bug: chromium:565742177
Change-Id: I63251adef97c03644d499ad95baa244033f6678e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/505661
Commit-Queue: Tomas Gunnarsson <[email protected]>
Reviewed-by: Stefan Holmer <[email protected]>
Cr-Commit-Position: refs/heads/main@{#48722}
---
diff --git a/pc/sdp_offer_answer.cc b/pc/sdp_offer_answer.cc
index 2b9bbc7..5cf98d1 100644
--- a/pc/sdp_offer_answer.cc
+++ b/pc/sdp_offer_answer.cc
@@ -2798,7 +2798,8 @@
       // direction.
       transceiver->set_current_direction(local_direction);
       // 2.2.8.1.11.[3-6]: Set the transport internal slots.
-      if (transceiver->mid()) {
+      // A stopped transceiver has no channel and therefore no transport.
+      if (transceiver->mid() && !transceiver->stopped()) {
         auto it = dtls_transports_by_mid.find(*transceiver->mid());
         RTC_DCHECK(it != dtls_transports_by_mid.end());
         transceiver->SetTransport(it->second.transport,
@@ -4899,7 +4900,8 @@
   TRACE_EVENT0("webrtc", "SdpOfferAnswerHandler::UpdateTransceiverChannel");
   RTC_DCHECK(IsUnifiedPlan());
   RTC_DCHECK(transceiver);
-  if (content.rejected) {
+  // A stopped transceiver must not have a channel.
+  if (content.rejected || transceiver->internal()->stopped()) {
     if (transceiver->internal()->HasChannel()) {
       network_teardown_tasks.Add(
           transceiver->internal()->GetClearChannelNetworkTask());
@@ -5993,6 +5995,10 @@
   }
   // Traverse a copy of the transceiver list.
   auto transceiver_list = transceivers()->List();
+  // Batched channel teardown for removed transceivers. The tasks run at the
+  // end of this function while `transceiver_list` still holds references.
+  ScopedOperationsBatcher network_teardown_tasks(context_->network_thread());
+  ScopedOperationsBatcher worker_tasks(context_->worker_thread());
   for (const auto& transceiver : transceiver_list) {
     // 3.2.10.1.1: If transceiver is stopped, associated with an m= section
     //             and the associated m= section is rejected in
@@ -6018,8 +6024,19 @@
       RTC_LOG(LS_INFO)
           << "Dropping stopped transceiver that was never associated";
     }
+    // Make sure the channel is cleared before the transceiver is removed.
+    if (transceiver->internal()->HasChannel()) {
+      network_teardown_tasks.Add(
+          transceiver->internal()->GetClearChannelNetworkTask());
+      worker_tasks.Add(transceiver->internal()->GetDeleteChannelWorkerTask(
+          /*stop_senders=*/false));
+    }
     transceivers()->Remove(transceiver);
   }
+  RTCError error = network_teardown_tasks.Run();
+  RTC_DCHECK(error.ok());
+  error = worker_tasks.Run();
+  RTC_DCHECK(error.ok());
 }
 
 void SdpOfferAnswerHandler::RemoveUnusedChannels(
diff --git a/pc/sdp_offer_answer_unittest.cc b/pc/sdp_offer_answer_unittest.cc
index 562cb0a..82bda92 100644
--- a/pc/sdp_offer_answer_unittest.cc
+++ b/pc/sdp_offer_answer_unittest.cc
@@ -48,8 +48,10 @@
 #include "media/base/codec.h"
 #include "media/base/media_constants.h"
 #include "media/base/stream_params.h"
+#include "p2p/base/p2p_constants.h"
 #include "pc/peer_connection_wrapper.h"
 #include "pc/rtp_transceiver.h"
+#include "pc/sdp_utils.h"
 #include "pc/session_description.h"
 #include "pc/test/fake_audio_capture_module.h"
 #include "pc/test/integration_test_helpers.h"
@@ -2341,6 +2343,51 @@
   EXPECT_FALSE(pc1->SetRemoteDescription(std::move(answer3)));
 }
 
+// Verifies that a stopped transceiver does not get a channel.
+TEST_F(SdpOfferAnswerTest, StoppedTransceiverHasNoChannel) {
+  auto caller = CreatePeerConnection();
+  auto callee = CreatePeerConnection();
+
+  caller->AddTransceiver(MediaType::AUDIO);
+  scoped_refptr<RtpTransceiverInterface> stopped_transceiver =
+      caller->AddTransceiver(MediaType::AUDIO);
+  ASSERT_TRUE(stopped_transceiver);
+
+  auto offer = caller->CreateOfferAndSetAsLocal();
+  ASSERT_THAT(offer, NotNull());
+  ASSERT_TRUE(callee->SetRemoteDescription(std::move(offer)));
+  auto answer = callee->CreateAnswerAndSetAsLocal();
+  ASSERT_THAT(answer, NotNull());
+
+  auto pranswer =
+      CloneSessionDescriptionAsType(answer.get(), SdpType::kPrAnswer);
+  ASSERT_THAT(pranswer, NotNull());
+  SessionDescription* pranswer_desc = pranswer->description();
+  ASSERT_THAT(pranswer_desc->contents(), SizeIs(2));
+  ContentInfo& rejected_content = pranswer_desc->contents()[1];
+  rejected_content.rejected = true;
+  const ContentGroup* bundle = pranswer_desc->GetGroupByName(GROUP_TYPE_BUNDLE);
+  ASSERT_THAT(bundle, NotNull());
+  ContentGroup new_bundle = *bundle;
+  new_bundle.RemoveContentName(rejected_content.mid());
+  pranswer_desc->RemoveGroupByName(GROUP_TYPE_BUNDLE);
+  pranswer_desc->AddGroup(new_bundle);
+
+  ASSERT_TRUE(caller->SetRemoteDescription(std::move(pranswer)));
+  EXPECT_TRUE(stopped_transceiver->stopped());
+
+  ASSERT_TRUE(caller->SetRemoteDescription(std::move(answer)));
+  EXPECT_TRUE(stopped_transceiver->stopped());
+
+  bool has_channel = signaling_thread_->BlockingCall([&] {
+    return static_cast<RtpTransceiverProxyWithInternal<RtpTransceiver>*>(
+               stopped_transceiver.get())
+        ->internal()
+        ->HasChannel();
+  });
+  EXPECT_FALSE(has_channel);
+}
+
 TEST_F(SdpOfferAnswerTest, SubsequentOfferDoesNotAddSctpInit) {
   auto pc1 = CreatePeerConnection("WebRTC-Sctp-Snap/Enabled/");
   auto pc2 = CreatePeerConnection("WebRTC-Sctp-Snap/Enabled/");
diff --git a/pc/BUILD.gn b/pc/BUILD.gn
index 9f715cb..e71364f 100644
--- a/pc/BUILD.gn
+++ b/pc/BUILD.gn
@@ -3815,6 +3815,7 @@
       ":pc_test_utils",
       ":peerconnection_wrapper",
       ":rtp_transceiver",
+      ":sdp_utils",
       ":session_description",
       "../api:create_peerconnection_factory",
       "../api:jsep",
@@ -3841,6 +3842,7 @@
       "../media:codec",
       "../media:media_constants",
       "../media:stream_params",
+      "../p2p:p2p_constants",
       "../rtc_base:ssl_adapter",
       "../rtc_base:threading",
       "../system_wrappers:metrics",

diff --git a/pc/sdp_offer_answer_unittest.cc b/pc/sdp_offer_answer_unittest.cc
index 562cb0a..82bda92 100644
--- a/pc/sdp_offer_answer_unittest.cc
+++ b/pc/sdp_offer_answer_unittest.cc
@@ -48,8 +48,10 @@
 #include "media/base/codec.h"
 #include "media/base/media_constants.h"
 #include "media/base/stream_params.h"
+#include "p2p/base/p2p_constants.h"
 #include "pc/peer_connection_wrapper.h"
 #include "pc/rtp_transceiver.h"
+#include "pc/sdp_utils.h"
 #include "pc/session_description.h"
 #include "pc/test/fake_audio_capture_module.h"
 #include "pc/test/integration_test_helpers.h"
@@ -2341,6 +2343,51 @@
   EXPECT_FALSE(pc1->SetRemoteDescription(std::move(answer3)));
 }
 
+// Verifies that a stopped transceiver does not get a channel.
+TEST_F(SdpOfferAnswerTest, StoppedTransceiverHasNoChannel) {
+  auto caller = CreatePeerConnection();
+  auto callee = CreatePeerConnection();
+
+  caller->AddTransceiver(MediaType::AUDIO);
+  scoped_refptr<RtpTransceiverInterface> stopped_transceiver =
+      caller->AddTransceiver(MediaType::AUDIO);
+  ASSERT_TRUE(stopped_transceiver);
+
+  auto offer = caller->CreateOfferAndSetAsLocal();
+  ASSERT_THAT(offer, NotNull());
+  ASSERT_TRUE(callee->SetRemoteDescription(std::move(offer)));
+  auto answer = callee->CreateAnswerAndSetAsLocal();
+  ASSERT_THAT(answer, NotNull());
+
+  auto pranswer =
+      CloneSessionDescriptionAsType(answer.get(), SdpType::kPrAnswer);
+  ASSERT_THAT(pranswer, NotNull());
+  SessionDescription* pranswer_desc = pranswer->description();
+  ASSERT_THAT(pranswer_desc->contents(), SizeIs(2));
+  ContentInfo& rejected_content = pranswer_desc->contents()[1];
+  rejected_content.rejected = true;
+  const ContentGroup* bundle = pranswer_desc->GetGroupByName(GROUP_TYPE_BUNDLE);
+  ASSERT_THAT(bundle, NotNull());
+  ContentGroup new_bundle = *bundle;
+  new_bundle.RemoveContentName(rejected_content.mid());
+  pranswer_desc->RemoveGroupByName(GROUP_TYPE_BUNDLE);
+  pranswer_desc->AddGroup(new_bundle);
+
+  ASSERT_TRUE(caller->SetRemoteDescription(std::move(pranswer)));
+  EXPECT_TRUE(stopped_transceiver->stopped());
+
+  ASSERT_TRUE(caller->SetRemoteDescription(std::move(answer)));
+  EXPECT_TRUE(stopped_transceiver->stopped());
+
+  bool has_channel = signaling_thread_->BlockingCall([&] {
+    return static_cast<RtpTransceiverProxyWithInternal<RtpTransceiver>*>(
+               stopped_transceiver.get())
+        ->internal()
+        ->HasChannel();
+  });
+  EXPECT_FALSE(has_channel);
+}
+
 TEST_F(SdpOfferAnswerTest, SubsequentOfferDoesNotAddSctpInit) {
   auto pc1 = CreatePeerConnection("WebRTC-Sctp-Snap/Enabled/");
   auto pc2 = CreatePeerConnection("WebRTC-Sctp-Snap/Enabled/");