commit 2919075ce36c8a040dff69f0e4382cd0bcd63196
author Tommi <tommi@webrtc.org> Sat Oct 21 14:50:09 2023
committer WebRTC LUCI CQ <webrtc-scoped@luci-project-accounts.iam.gserviceaccount.com> Sat Oct 21 16:39:05 2023
tree ace7e8c5c3a0b7f43c610d0cb49b10fd8d77162b
parent 840cf786002f0dd7cc3e9c1ce6ff5f3d5048d836

Remove an invoke for datahannel transport uninitialization during Close.

Bug: none
Change-Id: Ic0d482a8a045d3aa0fcaf13e43f8a156fa3560d6
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/324301
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40982}

pc/peer_connection.cc

diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc
index c31f671..a8d1d70 100644
--- a/pc/peer_connection.cc
+++ b/pc/peer_connection.cc
@@ -566,10 +566,8 @@
   if (sdp_handler_) {
     // Don't destroy BaseChannels until after stats has been cleaned up so that
     // the last stats request can still read from the channels.
-    sdp_handler_->DestroyAllChannels();
-
+    sdp_handler_->DestroyMediaChannels();
     RTC_LOG(LS_INFO) << "Session: " << session_id() << " is destroyed.";
-
     sdp_handler_->ResetSessionDescFactory();
   }
 
@@ -584,6 +582,8 @@
     if (network_thread_safety_)
       network_thread_safety_->SetNotAlive();
   });
+  sctp_mid_s_.reset();
+  SetSctpTransportName("");
 
   // call_ and event_log_ must be destroyed on the worker thread.
   worker_thread()->BlockingCall([this] {
@@ -1899,7 +1899,12 @@
 
   // Don't destroy BaseChannels until after stats has been cleaned up so that
   // the last stats request can still read from the channels.
-  sdp_handler_->DestroyAllChannels();
+  // TODO(tommi): The voice/video channels will be partially uninitialized on
+  // the network thread (see `RtpTransceiver::ClearChannel`), partially on the
+  // worker thread (see `PushNewMediaChannelAndDeleteChannel`) and then
+  // eventually freed on the signaling thread.
+  // It would be good to combine those steps with the teardown steps here.
+  sdp_handler_->DestroyMediaChannels();
 
   // The event log is used in the transport controller, which must be outlived
   // by the former. CreateOffer by the peer connection is implemented
@@ -1912,12 +1917,8 @@
   }
 
   network_thread()->BlockingCall([this] {
-    // Data channels will already have been unset via the DestroyAllChannels()
-    // call above, which triggers a call to TeardownDataChannelTransport_n().
-    // TODO(tommi): ^^ That's not exactly optimal since this is yet another
-    // blocking hop to the network thread during Close(). Further still, the
-    // voice/video/data channels will be cleared on the worker thread.
     RTC_DCHECK_RUN_ON(network_thread());
+    TeardownDataChannelTransport_n({});
     transport_controller_.reset();
     port_allocator_->DiscardCandidatePool();
     if (network_thread_safety_) {
@@ -1925,6 +1926,9 @@
     }
   });
 
+  sctp_mid_s_.reset();
+  SetSctpTransportName("");
+
   worker_thread()->BlockingCall([this] {
     RTC_DCHECK_RUN_ON(worker_thread());
     worker_thread_safety_->SetNotAlive();

pc/sdp_offer_answer.cc

diff --git a/pc/sdp_offer_answer.cc b/pc/sdp_offer_answer.cc
index c795ccf..0261195 100644
--- a/pc/sdp_offer_answer.cc
+++ b/pc/sdp_offer_answer.cc
@@ -5204,7 +5204,7 @@
   return RTCError::OK();
 }
 
-void SdpOfferAnswerHandler::DestroyAllChannels() {
+void SdpOfferAnswerHandler::DestroyMediaChannels() {
   RTC_DCHECK_RUN_ON(signaling_thread());
   if (!transceivers()) {
     return;
@@ -5227,8 +5227,6 @@
       transceiver->internal()->ClearChannel();
     }
   }
-
-  pc_->DestroyDataChannelTransport({});
 }
 
 void SdpOfferAnswerHandler::GenerateMediaDescriptionOptions(

pc/sdp_offer_answer.h

diff --git a/pc/sdp_offer_answer.h b/pc/sdp_offer_answer.h
index eb7c705..8aa7040 100644
--- a/pc/sdp_offer_answer.h
+++ b/pc/sdp_offer_answer.h
@@ -166,8 +166,8 @@
   // See also `InternalDataChannelInit::fallback_ssl_role`.
   absl::optional<rtc::SSLRole> GuessSslRole() const;
 
-  // Destroys all BaseChannels and destroys the SCTP data channel, if present.
-  void DestroyAllChannels();
+  // Destroys all media BaseChannels.
+  void DestroyMediaChannels();
 
   rtc::scoped_refptr<StreamCollectionInterface> local_streams();
   rtc::scoped_refptr<StreamCollectionInterface> remote_streams();