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

Transition data channnel to kClosed when not connected to a transport.

Browse Source 
If a data channel object was closed (via calling Close()) right after
construction and before attaching to a transport, it would never
transition to the `kClosed` state. This addresses that corner case,
which caused a DCHECK to trigger but might also cause a situation
whereby more than one DC instance existed for a given sctp sid.

Bug: chromium:1421534
Change-Id: Id757c0528f929f2e2daa5343236d7f62e309f6cc
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/296341
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39513}
This commit is contained in:
Tommi 2023-03-07 08:43:24 +01:00 committed by WebRTC LUCI CQ
parent 5a54800957
commit f21354ce0e
2 changed files with 27 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
pc/data_channel_unittest.cc
View File

@ -618,6 +618,15 @@ TEST_F(SctpDataChannelTest, NeverOpened) {
webrtc_data_channel_->Close();
}
// Tests that a data channel that's not connected to a transport can transition
// directly to the `kClosed` state when closed.
// See also chromium:1421534.
TEST_F(SctpDataChannelTest, UnusedTransitionsDirectlyToClosed) {
webrtc_data_channel_->Close();
EXPECT_EQ(webrtc::DataChannelInterface::kClosed,
webrtc_data_channel_->state());
}
// Test that the data channel goes to the "closed" state (and doesn't crash)
// when its transport goes away, even while data is buffered.
TEST_F(SctpDataChannelTest, TransportDestroyedWhileDataBuffered) {

29
pc/sctp_data_channel.cc
View File

@ -539,18 +539,25 @@ void SctpDataChannel::UpdateState() {
break;
}
case kClosing: {
// Wait for all queued data to be sent before beginning the closing
// procedure.
if (queued_send_data_.Empty() && queued_control_data_.Empty()) {
// For SCTP data channels, we need to wait for the closing procedure
// to complete; after calling RemoveSctpDataStream,
// OnClosingProcedureComplete will end up called asynchronously
// afterwards.
if (connected_to_transport_ && !started_closing_procedure_ &&
controller_ && config_.id >= 0) {
started_closing_procedure_ = true;
controller_->RemoveSctpDataStream(config_.id);
if (connected_to_transport_) {
// Wait for all queued data to be sent before beginning the closing
// procedure.
if (queued_send_data_.Empty() && queued_control_data_.Empty()) {
// For SCTP data channels, we need to wait for the closing procedure
// to complete; after calling RemoveSctpDataStream,
// OnClosingProcedureComplete will end up called asynchronously
// afterwards.
if (!started_closing_procedure_ && controller_ && config_.id >= 0) {
started_closing_procedure_ = true;
controller_->RemoveSctpDataStream(config_.id);
}
}
} else {
// When we're not connected to a transport, we'll transition
// directly to the `kClosed` state from here.
queued_send_data_.Clear();
queued_control_data_.Clear();
SetState(kClosed);
}
break;
}