admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Sort threading for sctp_mid_ variable

Browse Source 
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}
This commit is contained in:
Harald Alvestrand 2020-02-12 07:38:21 +01:00 committed by Commit Bot
parent bc1750d52b
commit 7a829a8563
2 changed files with 28 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

41
pc/peer_connection.cc
View File

@ -4363,10 +4363,10 @@ PeerConnection::LookupDtlsTransportByMidInternal(const std::string& mid) {
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 @@ absl::optional<std::string> PeerConnection::GetDataMid() const {
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 @@ bool PeerConnection::GetSctpSslRole(rtc::SSLRole* role) {
}
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 @@ RTCError PeerConnection::PushdownMediaDescription(
// 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 @@ cricket::IceConfig PeerConnection::ParseIceConfig(
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 @@ std::map<std::string, std::string> PeerConnection::GetTransportNamesByMid()
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 @@ bool PeerConnection::CreateDataChannel(const std::string& mid) {
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 @@ bool PeerConnection::SetupDataChannelTransport_n(const std::string& mid) {
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 @@ bool PeerConnection::SetupDataChannelTransport_n(const std::string& mid) {
}
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 @@ const std::string PeerConnection::GetTransportName(
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 @@ void PeerConnection::DestroyDataChannelTransport() {
// 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 @@ bool PeerConnection::OnTransportChanged(
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;

10
pc/peer_connection.h
View File

@ -321,7 +321,7 @@ class PeerConnection : public PeerConnectionInternal,
void RequestUsagePatternReportForTesting();
absl::optional<std::string> sctp_mid() {
RTC_DCHECK_RUN_ON(signaling_thread());
return sctp_mid_;
return sctp_mid_s_;
}
protected:
@ -1330,9 +1330,11 @@ class PeerConnection : public PeerConnectionInternal,
// Note: this is used as the data channel MID by both SCTP and data channel
// transports. It is set when either transport is initialized and unset when
// both transports are deleted.
absl::optional<std::string>
sctp_mid_; // TODO(bugs.webrtc.org/9987): Accessed on both signaling
// and network thread.
// There is one copy on the signaling thread and another copy on the
// networking thread. Changes are always initiated from the signaling
// thread, but applied first on the networking thread via an invoke().
absl::optional<std::string> sctp_mid_s_ RTC_GUARDED_BY(signaling_thread());
absl::optional<std::string> sctp_mid_n_ RTC_GUARDED_BY(network_thread());
// Whether this peer is the caller. Set when the local description is applied.
absl::optional<bool> is_caller_ RTC_GUARDED_BY(signaling_thread());