zqz/platform-external-webrtc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

dcsctp: Avoid transition back to ShutdownPending

Browse Source 
When a socket is shutting down, either explicitly by the ULP calling
Shutdown(), or when the socket receives a SHUTDOWN chunk, the socket
should send all outstanding data and when all is sent and acked,
_then_ it should continue the shutdown protocol.

As it currently doesn't calculate correctly when all data has been sent,
as NACKED chunks are not included in what it believes is remaining in
the retransmission queue, it will shut down prematurely and may go back
to a previous state (ShutdownPending) from ShutdownSent or
ShutdownAckSent.

This is a workaround that just avoids the illegal state transition as
that puts the socket in an inconsistent state. The bug is merely
theoretical as WebRTC doesn't currently gracefully shut down a SCTP
socket, but just terminates the DTLS transport.

As TODOs mention, this will be fixed correctly a bit later.

Bug: webrtc:12739
Change-Id: Ibde2acc3a6aca701ac178d6181028404d470a5d5
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/218340
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33982}
This commit is contained in:
Victor Boivie
2021-05-06 21:07:49 +02:00
committed by WebRTC LUCI CQ
parent 78d5859bde
commit 50a0b1219e
1 changed files with 14 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files

18
net/dcsctp/socket/dcsctp_socket.cc
View File

@ -279,10 +279,16 @@ void DcSctpSocket::Shutdown() {
// "Upon receipt of the SHUTDOWN primitive from its upper layer, the
// endpoint enters the SHUTDOWN-PENDING state and remains there until all
// outstanding data has been acknowledged by its peer."
SetState(State::kShutdownPending, "Shutdown called");
t1_init_->Stop();
t1_cookie_->Stop();
MaybeSendShutdownOrAck();
// TODO(webrtc:12739): Remove this check, as it just hides the problem that
// the socket can transition from ShutdownSent to ShutdownPending, or
// ShutdownAckSent to ShutdownPending which is illegal.
if (state_ != State::kShutdownSent && state_ != State::kShutdownAckSent) {
SetState(State::kShutdownPending, "Shutdown called");
t1_init_->Stop();
t1_cookie_->Stop();
MaybeSendShutdownOrAck();
}
} else {
// Connection closed before even starting to connect, or during the initial
// connection phase. There is no outstanding data, so the socket can just
@ -1368,6 +1374,10 @@ void DcSctpSocket::HandleShutdown(
// state restarting its T2-shutdown timer."
SendShutdownAck();
SetState(State::kShutdownAckSent, "SHUTDOWN received");
} else if (state_ == State::kShutdownAckSent) {
// TODO(webrtc:12739): This condition should be removed and handled by the
// next (state_ != State::kShutdownReceived).
return;
} else if (state_ != State::kShutdownReceived) {
RTC_DLOG(LS_VERBOSE) << log_prefix()
<< "Received SHUTDOWN - shutting down the socket";