From dd1eb2e1ece1db1e0f221cca0317014028756967 Mon Sep 17 00:00:00 2001 From: Victor Boivie Date: Thu, 1 Sep 2022 13:00:28 +0000 Subject: [PATCH] dcsctp: Send buffered data directly on response When a stream reset response has been received, this may result in unpausing the streams (either because it was successful or because it failed - but streams will be unpaused). Directly after receiving the response, send out any pending chunks that are ready to be sent. Before this CL, they would eventually be sent out, but that is unnecessary and undeterministic. Bug: webrtc:14277 Change-Id: Ic1ab38bc3cea96cfec7419e25001f12807523a3a Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/273800 Commit-Queue: Victor Boivie Reviewed-by: Florent Castelli Cr-Commit-Position: refs/heads/main@{#38009} --- net/dcsctp/socket/dcsctp_socket.cc | 4 +++ net/dcsctp/socket/dcsctp_socket_test.cc | 34 +++++++++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/net/dcsctp/socket/dcsctp_socket.cc b/net/dcsctp/socket/dcsctp_socket.cc index d77e591c7b..f36e26d777 100644 --- a/net/dcsctp/socket/dcsctp_socket.cc +++ b/net/dcsctp/socket/dcsctp_socket.cc @@ -1541,6 +1541,7 @@ void DcSctpSocket::HandleError(const CommonHeader& header, void DcSctpSocket::HandleReconfig( const CommonHeader& header, const SctpPacket::ChunkDescriptor& descriptor) { + TimeMs now = callbacks_.TimeMillis(); absl::optional chunk = ReConfigChunk::Parse(descriptor.data); if (ValidateParseSuccess(chunk) && ValidateHasTCB()) { tcb_->stream_reset_handler().HandleReConfig(*std::move(chunk)); @@ -1549,6 +1550,9 @@ void DcSctpSocket::HandleReconfig( // that were waiting for this request to finish, continue resetting them. MaybeSendResetStreamsRequest(); } + // If a response was processed, pending to-be-reset streams may now have + // become unpaused. Try to send more DATA chunks. + tcb_->SendBufferedPackets(now); } void DcSctpSocket::HandleShutdown( diff --git a/net/dcsctp/socket/dcsctp_socket_test.cc b/net/dcsctp/socket/dcsctp_socket_test.cc index 46ac4bd183..f38624d606 100644 --- a/net/dcsctp/socket/dcsctp_socket_test.cc +++ b/net/dcsctp/socket/dcsctp_socket_test.cc @@ -2815,5 +2815,39 @@ TEST(DcSctpSocketTest, ResetStreamsDeferred) { EXPECT_EQ(msg3->payload().size(), kSmallMessageSize); } +TEST(DcSctpSocketTest, ResetStreamsWithPausedSenderResumesWhenPerformed) { + SocketUnderTest a("A"); + SocketUnderTest z("Z"); + + ConnectSockets(a, z); + + a.socket.Send(DcSctpMessage(StreamID(1), PPID(51), + std::vector(kSmallMessageSize)), + {}); + + a.socket.ResetStreams(std::vector({StreamID(1)})); + + // Will be queued, as the stream has an outstanding reset operation. + a.socket.Send(DcSctpMessage(StreamID(1), PPID(52), + std::vector(kSmallMessageSize)), + {}); + + EXPECT_CALL(a.cb, OnStreamsResetPerformed(ElementsAre(StreamID(1)))); + EXPECT_CALL(z.cb, OnIncomingStreamsReset(ElementsAre(StreamID(1)))); + ExchangeMessages(a, z); + + absl::optional msg1 = z.cb.ConsumeReceivedMessage(); + ASSERT_TRUE(msg1.has_value()); + EXPECT_EQ(msg1->stream_id(), StreamID(1)); + EXPECT_EQ(msg1->ppid(), PPID(51)); + EXPECT_EQ(msg1->payload().size(), kSmallMessageSize); + + absl::optional msg2 = z.cb.ConsumeReceivedMessage(); + ASSERT_TRUE(msg2.has_value()); + EXPECT_EQ(msg2->stream_id(), StreamID(1)); + EXPECT_EQ(msg2->ppid(), PPID(52)); + EXPECT_EQ(msg2->payload().size(), kSmallMessageSize); +} + } // namespace } // namespace dcsctp