commit 7a829a85638571abcf62b284fc0f884aaeb5d428
author Harald Alvestrand <hta@webrtc.org> Wed Feb 12 06:38:21 2020
committer Commit Bot <commit-bot@chromium.org> Wed Feb 12 08:34:12 2020
tree a69d0fcba2c9c49355cf5ce0a3fcbc9a13c5d2d0
parent bc1750d52b93da9651c0247cf64d5e0155ca281f

Sort threading for sctp_mid_ variable

Split the sctp_mid_ variable into two variables,
sctp_mid_n_ and sctp_mid_s_, each of which is only accessed
by one thread.

Bug: webrtc:9987
Change-Id: I4dce944b920f4698e2606a7b85776791cbf55c28
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/168243
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#30503}

pc/peer_connection.cc

diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc
index 2e138c4..b9e9d29 100644
--- a/pc/peer_connection.cc
+++ b/pc/peer_connection.cc
@@ -4363,10 +4363,10 @@
 rtc::scoped_refptr<SctpTransportInterface> PeerConnection::GetSctpTransport()
     const {
   RTC_DCHECK_RUN_ON(signaling_thread());
-  if (!sctp_mid_) {
+  if (!sctp_mid_s_) {
     return nullptr;
   }
-  return transport_controller_->GetSctpTransport(*sctp_mid_);
+  return transport_controller_->GetSctpTransport(*sctp_mid_s_);
 }
 
 const SessionDescriptionInterface* PeerConnection::local_description() const {
@@ -5400,7 +5400,7 @@
     case cricket::DCT_SCTP:
     case cricket::DCT_DATA_CHANNEL_TRANSPORT:
     case cricket::DCT_DATA_CHANNEL_TRANSPORT_SCTP:
-      return sctp_mid_;
+      return sctp_mid_s_;
     default:
       return absl::nullopt;
   }
@@ -5927,8 +5927,8 @@
   }
 
   absl::optional<rtc::SSLRole> dtls_role;
-  if (sctp_mid_) {
-    dtls_role = transport_controller_->GetDtlsRole(*sctp_mid_);
+  if (sctp_mid_s_) {
+    dtls_role = transport_controller_->GetDtlsRole(*sctp_mid_s_);
     if (!dtls_role && is_caller_.has_value()) {
       dtls_role = *is_caller_ ? rtc::SSL_SERVER : rtc::SSL_CLIENT;
     }
@@ -6060,9 +6060,9 @@
 
   // Need complete offer/answer with an SCTP m= section before starting SCTP,
   // according to https://tools.ietf.org/html/draft-ietf-mmusic-sctp-sdp-19
-  if (sctp_mid_ && local_description() && remote_description()) {
+  if (sctp_mid_s_ && local_description() && remote_description()) {
     rtc::scoped_refptr<SctpTransport> sctp_transport =
-        transport_controller_->GetSctpTransport(*sctp_mid_);
+        transport_controller_->GetSctpTransport(*sctp_mid_s_);
     auto local_sctp_description = cricket::GetFirstSctpDataContentDescription(
         local_description()->description());
     auto remote_sctp_description = cricket::GetFirstSctpDataContentDescription(
@@ -6163,8 +6163,8 @@
 
 absl::optional<std::string> PeerConnection::sctp_transport_name() const {
   RTC_DCHECK_RUN_ON(signaling_thread());
-  if (sctp_mid_ && transport_controller_) {
-    auto dtls_transport = transport_controller_->GetDtlsTransport(*sctp_mid_);
+  if (sctp_mid_s_ && transport_controller_) {
+    auto dtls_transport = transport_controller_->GetDtlsTransport(*sctp_mid_s_);
     if (dtls_transport) {
       return dtls_transport->transport_name();
     }
@@ -6201,7 +6201,7 @@
   if (data_channel_controller_.data_channel_transport()) {
     absl::optional<std::string> transport_name = sctp_transport_name();
     RTC_DCHECK(transport_name);
-    transport_names_by_mid[*sctp_mid_] = *transport_name;
+    transport_names_by_mid[*sctp_mid_s_] = *transport_name;
   }
   return transport_names_by_mid;
 }
@@ -6633,10 +6633,12 @@
     case cricket::DCT_SCTP:
     case cricket::DCT_DATA_CHANNEL_TRANSPORT_SCTP:
     case cricket::DCT_DATA_CHANNEL_TRANSPORT:
-      if (!network_thread()->Invoke<bool>(
+      if (network_thread()->Invoke<bool>(
               RTC_FROM_HERE,
               rtc::Bind(&PeerConnection::SetupDataChannelTransport_n, this,
                         mid))) {
+        sctp_mid_s_ = mid;
+      } else {
         return false;
       }
 
@@ -6694,7 +6696,7 @@
 
   data_channel_controller_.set_data_channel_transport(transport);
   data_channel_controller_.SetupDataChannelTransport_n();
-  sctp_mid_ = mid;
+  sctp_mid_n_ = mid;
 
   // Note: setting the data sink and checking initial state must be done last,
   // after setting up the data channel.  Setting the data sink may trigger
@@ -6705,15 +6707,15 @@
 }
 
 void PeerConnection::TeardownDataChannelTransport_n() {
-  if (!sctp_mid_ && !data_channel_controller_.data_channel_transport()) {
+  if (!sctp_mid_n_ && !data_channel_controller_.data_channel_transport()) {
     return;
   }
   RTC_LOG(LS_INFO) << "Tearing down data channel transport for mid="
-                   << *sctp_mid_;
+                   << *sctp_mid_n_;
 
   // |sctp_mid_| may still be active through an SCTP transport.  If not, unset
   // it.
-  sctp_mid_.reset();
+  sctp_mid_n_.reset();
   data_channel_controller_.TeardownDataChannelTransport_n();
 }
 
@@ -7234,8 +7236,8 @@
     return channel->transport_name();
   }
   if (data_channel_controller_.data_channel_transport()) {
-    RTC_DCHECK(sctp_mid_);
-    if (content_name == *sctp_mid_) {
+    RTC_DCHECK(sctp_mid_s_);
+    if (content_name == *sctp_mid_s_) {
       return *sctp_transport_name();
     }
   }
@@ -7268,12 +7270,13 @@
   // been destroyed (since it is a subclass of PeerConnection) and using
   // rtc::Bind will cause "Pure virtual function called" error to appear.
 
-  if (sctp_mid_) {
+  if (sctp_mid_s_) {
     data_channel_controller_.OnTransportChannelClosed();
     network_thread()->Invoke<void>(RTC_FROM_HERE, [this] {
       RTC_DCHECK_RUN_ON(network_thread());
       TeardownDataChannelTransport_n();
     });
+    sctp_mid_s_.reset();
   }
 }
 
@@ -7310,7 +7313,7 @@
   if (base_channel) {
     ret = base_channel->SetRtpTransport(rtp_transport);
   }
-  if (mid == sctp_mid_) {
+  if (mid == sctp_mid_n_) {
     data_channel_controller_.OnTransportChanged(data_channel_transport);
   }
   return ret;