From f95536dd5a39e7bef6ddcd2f507a63064e07c8cc Mon Sep 17 00:00:00 2001 From: Victor Boivie Date: Fri, 7 May 2021 10:56:52 +0200 Subject: [PATCH] dcsctp: Stop connection timers during shutdown If Shutdown is called when the socket is being established and while the connection timers are running, it will put the socket in an inconsistent state, which is verified in debug builds. Bug: webrtc:12614 Change-Id: I66f07d1170ac8f0ad9fd485d77d6aef4c365f150 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/217765 Commit-Queue: Victor Boivie Reviewed-by: Florent Castelli Cr-Commit-Position: refs/heads/master@{#33949} --- net/dcsctp/socket/dcsctp_socket.cc | 2 ++ net/dcsctp/socket/dcsctp_socket_test.cc | 40 +++++++++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/net/dcsctp/socket/dcsctp_socket.cc b/net/dcsctp/socket/dcsctp_socket.cc index dd54317718..622f6ecfcf 100644 --- a/net/dcsctp/socket/dcsctp_socket.cc +++ b/net/dcsctp/socket/dcsctp_socket.cc @@ -280,6 +280,8 @@ void DcSctpSocket::Shutdown() { // 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(); } else { // Connection closed before even starting to connect, or during the initial diff --git a/net/dcsctp/socket/dcsctp_socket_test.cc b/net/dcsctp/socket/dcsctp_socket_test.cc index 2b2e9880f1..a9eec60083 100644 --- a/net/dcsctp/socket/dcsctp_socket_test.cc +++ b/net/dcsctp/socket/dcsctp_socket_test.cc @@ -282,6 +282,46 @@ TEST_F(DcSctpSocketTest, EstablishConnectionWithSetupCollision) { EXPECT_EQ(sock_z_.state(), SocketState::kConnected); } +TEST_F(DcSctpSocketTest, ShuttingDownWhileEstablishingConnection) { + EXPECT_CALL(cb_a_, OnConnected).Times(0); + EXPECT_CALL(cb_z_, OnConnected).Times(1); + sock_a_.Connect(); + + // Z reads INIT, produces INIT_ACK + sock_z_.ReceivePacket(cb_a_.ConsumeSentPacket()); + // A reads INIT_ACK, produces COOKIE_ECHO + sock_a_.ReceivePacket(cb_z_.ConsumeSentPacket()); + // Z reads COOKIE_ECHO, produces COOKIE_ACK + sock_z_.ReceivePacket(cb_a_.ConsumeSentPacket()); + // Drop COOKIE_ACK, just to more easily verify shutdown protocol. + cb_z_.ConsumeSentPacket(); + + // As Socket A has received INIT_ACK, it has a TCB and is connected, while + // Socket Z needs to receive COOKIE_ECHO to get there. Socket A still has + // timers running at this point. + EXPECT_EQ(sock_a_.state(), SocketState::kConnecting); + EXPECT_EQ(sock_z_.state(), SocketState::kConnected); + + // Socket A is now shut down, which should make it stop those timers. + sock_a_.Shutdown(); + + EXPECT_CALL(cb_a_, OnClosed).Times(1); + EXPECT_CALL(cb_z_, OnClosed).Times(1); + + // Z reads SHUTDOWN, produces SHUTDOWN_ACK + sock_z_.ReceivePacket(cb_a_.ConsumeSentPacket()); + // A reads SHUTDOWN_ACK, produces SHUTDOWN_COMPLETE + sock_a_.ReceivePacket(cb_z_.ConsumeSentPacket()); + // Z reads SHUTDOWN_COMPLETE. + sock_z_.ReceivePacket(cb_a_.ConsumeSentPacket()); + + EXPECT_TRUE(cb_a_.ConsumeSentPacket().empty()); + EXPECT_TRUE(cb_z_.ConsumeSentPacket().empty()); + + EXPECT_EQ(sock_a_.state(), SocketState::kClosed); + EXPECT_EQ(sock_z_.state(), SocketState::kClosed); +} + TEST_F(DcSctpSocketTest, EstablishSimultaneousConnection) { EXPECT_CALL(cb_a_, OnConnected).Times(1); EXPECT_CALL(cb_z_, OnConnected).Times(1);