Make SCTPtransport enter "closed" state when DTLStransport does.
Bug: webrtc:11090
Change-Id: I30e0b70387746d6c544ed1818f276569d4258cf4
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/159888
Reviewed-by: Emad Omara <emadomara@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#29810}
diff --git a/p2p/base/fake_dtls_transport.h b/p2p/base/fake_dtls_transport.h
index cf6fd78..7061ea4 100644
--- a/p2p/base/fake_dtls_transport.h
+++ b/p2p/base/fake_dtls_transport.h
@@ -83,7 +83,10 @@
ice_transport_->SetReceiving(receiving);
set_receiving(receiving);
}
- void SetDtlsState(DtlsTransportState state) { dtls_state_ = state; }
+ void SetDtlsState(DtlsTransportState state) {
+ dtls_state_ = state;
+ SignalDtlsState(this, dtls_state_);
+ }
// Simulates the two DTLS transports connecting to each other.
// If |asymmetric| is true this method only affects this FakeDtlsTransport.
@@ -108,12 +111,11 @@
if (!asymmetric) {
dest->SetDestination(this, true);
}
- dtls_state_ = DTLS_TRANSPORT_CONNECTED;
// If the |dtls_role_| is unset, set it to SSL_CLIENT by default.
if (!dtls_role_) {
dtls_role_ = std::move(rtc::SSL_CLIENT);
}
- SignalDtlsState(this, dtls_state_);
+ SetDtlsState(DTLS_TRANSPORT_CONNECTED);
ice_transport_->SetDestination(
static_cast<FakeIceTransport*>(dest->ice_transport()), asymmetric);
} else {
diff --git a/pc/data_channel.cc b/pc/data_channel.cc
index c5a8aeb..e87bb85 100644
--- a/pc/data_channel.cc
+++ b/pc/data_channel.cc
@@ -359,9 +359,10 @@
}
}
-void DataChannel::OnTransportChannelDestroyed() {
- // The SctpTransport is going away (for example, because the SCTP m= section
- // was rejected), so we need to close abruptly.
+void DataChannel::OnTransportChannelClosed() {
+ // The SctpTransport is unusable (for example, because the SCTP m= section
+ // was rejected, or because the DTLS transport closed), so we need to close
+ // abruptly.
CloseAbruptly();
}
diff --git a/pc/data_channel.h b/pc/data_channel.h
index 728226c..c7dc6ea 100644
--- a/pc/data_channel.h
+++ b/pc/data_channel.h
@@ -185,10 +185,10 @@
// Called when the transport channel is created.
// Only needs to be called for SCTP data channels.
void OnTransportChannelCreated();
- // Called when the transport channel is destroyed.
+ // Called when the transport channel is unusable.
// This method makes sure the DataChannel is disconnected and changes state
// to kClosed.
- void OnTransportChannelDestroyed();
+ void OnTransportChannelClosed();
/*******************************************
* The following methods are for RTP only. *
diff --git a/pc/data_channel_unittest.cc b/pc/data_channel_unittest.cc
index ad0e9b6..46e1df2 100644
--- a/pc/data_channel_unittest.cc
+++ b/pc/data_channel_unittest.cc
@@ -610,10 +610,10 @@
provider_->set_send_blocked(true);
EXPECT_TRUE(webrtc_data_channel_->Send(packet));
- // Tell the data channel that its tranpsort is being destroyed.
+ // Tell the data channel that its transport is being destroyed.
// It should then stop using the transport (allowing us to delete it) and
// transition to the "closed" state.
- webrtc_data_channel_->OnTransportChannelDestroyed();
+ webrtc_data_channel_->OnTransportChannelClosed();
provider_.reset(nullptr);
EXPECT_EQ_WAIT(webrtc::DataChannelInterface::kClosed,
webrtc_data_channel_->state(), kDefaultTimeout);
diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc
index 1d7b4ea..9fac748 100644
--- a/pc/peer_connection.cc
+++ b/pc/peer_connection.cc
@@ -1106,7 +1106,7 @@
DestroyTransceiverChannel(transceiver);
}
}
- DestroyDataChannel();
+ DestroyDataChannelTransport();
}
bool PeerConnection::Initialize(
@@ -3597,7 +3597,7 @@
}
if (content.rejected) {
RTC_LOG(LS_INFO) << "Rejected data channel, mid=" << content.mid();
- DestroyDataChannel();
+ DestroyDataChannelTransport();
} else {
if (!rtp_data_channel_ && !data_channel_transport_) {
RTC_LOG(LS_INFO) << "Creating data channel, mid=" << content.mid();
@@ -5907,19 +5907,19 @@
}
}
-void PeerConnection::OnDataChannelDestroyed() {
+void PeerConnection::OnTransportChannelClosed() {
// Use a temporary copy of the RTP/SCTP DataChannel list because the
// DataChannel may callback to us and try to modify the list.
std::map<std::string, rtc::scoped_refptr<DataChannel>> temp_rtp_dcs;
temp_rtp_dcs.swap(rtp_data_channels_);
for (const auto& kv : temp_rtp_dcs) {
- kv.second->OnTransportChannelDestroyed();
+ kv.second->OnTransportChannelClosed();
}
std::vector<rtc::scoped_refptr<DataChannel>> temp_sctp_dcs;
temp_sctp_dcs.swap(sctp_data_channels_);
for (const auto& channel : temp_sctp_dcs) {
- channel->OnTransportChannelDestroyed();
+ channel->OnTransportChannelClosed();
}
}
@@ -6978,7 +6978,7 @@
const cricket::ContentInfo* data_info = cricket::GetFirstDataContent(desc);
if (!data_info || data_info->rejected) {
- DestroyDataChannel();
+ DestroyDataChannelTransport();
}
}
@@ -7709,9 +7709,9 @@
}
}
-void PeerConnection::DestroyDataChannel() {
+void PeerConnection::DestroyDataChannelTransport() {
if (rtp_data_channel_) {
- OnDataChannelDestroyed();
+ OnTransportChannelClosed();
DestroyChannelInterface(rtp_data_channel_);
rtp_data_channel_ = nullptr;
}
@@ -7723,7 +7723,7 @@
// rtc::Bind will cause "Pure virtual function called" error to appear.
if (sctp_mid_) {
- OnDataChannelDestroyed();
+ OnTransportChannelClosed();
network_thread()->Invoke<void>(RTC_FROM_HERE, [this] {
RTC_DCHECK_RUN_ON(network_thread());
TeardownDataChannelTransport_n();
diff --git a/pc/peer_connection.h b/pc/peer_connection.h
index cbff7e7..19af912 100644
--- a/pc/peer_connection.h
+++ b/pc/peer_connection.h
@@ -854,7 +854,8 @@
void OnSctpDataChannelClosed(DataChannel* channel)
RTC_RUN_ON(signaling_thread());
- void OnDataChannelDestroyed() RTC_RUN_ON(signaling_thread());
+ // Called when the transport for the data channels is closed or destroyed.
+ void OnTransportChannelClosed() RTC_RUN_ON(signaling_thread());
// Called when a valid data channel OPEN message is received.
void OnDataChannelOpenMessage(const std::string& label,
const InternalDataChannelInit& config)
@@ -1169,14 +1170,19 @@
const std::string GetTransportName(const std::string& content_name)
RTC_RUN_ON(signaling_thread());
+ // Functions for dealing with transports.
+ // Note that cricket code uses the term "channel" for what other code
+ // refers to as "transport".
+
// Destroys and clears the BaseChannel associated with the given transceiver,
// if such channel is set.
void DestroyTransceiverChannel(
rtc::scoped_refptr<RtpTransceiverProxyWithInternal<RtpTransceiver>>
transceiver);
- // Destroys the RTP data channel and/or the SCTP data channel and clears it.
- void DestroyDataChannel() RTC_RUN_ON(signaling_thread());
+ // Destroys the RTP data channel transport and/or the SCTP data channel
+ // transport and clears it.
+ void DestroyDataChannelTransport() RTC_RUN_ON(signaling_thread());
// Destroys the given ChannelInterface.
// The channel cannot be accessed after this method is called.
diff --git a/pc/sctp_transport.cc b/pc/sctp_transport.cc
index 6c4a8be..532e91c 100644
--- a/pc/sctp_transport.cc
+++ b/pc/sctp_transport.cc
@@ -86,6 +86,8 @@
if (internal_sctp_transport_) {
if (transport) {
internal_sctp_transport_->SetDtlsTransport(transport->internal());
+ transport->internal()->SignalDtlsState.connect(
+ this, &SctpTransport::OnDtlsStateChange);
if (info_.state() == SctpTransportState::kNew) {
next_state = SctpTransportState::kConnecting;
}
@@ -162,4 +164,15 @@
UpdateInformation(SctpTransportState::kConnected);
}
+void SctpTransport::OnDtlsStateChange(cricket::DtlsTransportInternal* transport,
+ cricket::DtlsTransportState state) {
+ RTC_DCHECK_RUN_ON(owner_thread_);
+ RTC_CHECK(transport == dtls_transport_->internal());
+ if (state == cricket::DTLS_TRANSPORT_CLOSED ||
+ state == cricket::DTLS_TRANSPORT_FAILED) {
+ UpdateInformation(SctpTransportState::kClosed);
+ // TODO(http://bugs.webrtc.org/11090): Close all the data channels
+ }
+}
+
} // namespace webrtc
diff --git a/pc/sctp_transport.h b/pc/sctp_transport.h
index c7727df..a13a58c 100644
--- a/pc/sctp_transport.h
+++ b/pc/sctp_transport.h
@@ -65,6 +65,8 @@
void OnAssociationChangeCommunicationUp();
void OnInternalClosingProcedureStartedRemotely(int sid);
void OnInternalClosingProcedureComplete(int sid);
+ void OnDtlsStateChange(cricket::DtlsTransportInternal* transport,
+ cricket::DtlsTransportState state);
// Note - owner_thread never changes, but can't be const if we do
// Invoke() on it.
diff --git a/pc/sctp_transport_unittest.cc b/pc/sctp_transport_unittest.cc
index 8566ef3..f3070cd 100644
--- a/pc/sctp_transport_unittest.cc
+++ b/pc/sctp_transport_unittest.cc
@@ -195,4 +195,17 @@
*(observer_.LastReceivedInformation().MaxChannels()));
}
+TEST_F(SctpTransportTest, CloseWhenTransportCloses) {
+ CreateTransport();
+ transport()->RegisterObserver(observer());
+ AddDtlsTransport();
+ CompleteSctpHandshake();
+ ASSERT_EQ_WAIT(SctpTransportState::kConnected, observer_.State(),
+ kDefaultTimeout);
+ static_cast<cricket::FakeDtlsTransport*>(dtls_transport_->internal())
+ ->SetDtlsState(cricket::DTLS_TRANSPORT_CLOSED);
+ ASSERT_EQ_WAIT(SctpTransportState::kClosed, observer_.State(),
+ kDefaultTimeout);
+}
+
} // namespace webrtc
