diff --git a/net/dcsctp/socket/dcsctp_socket.cc b/net/dcsctp/socket/dcsctp_socket.cc index d197a38386..48b6edb67f 100644 --- a/net/dcsctp/socket/dcsctp_socket.cc +++ b/net/dcsctp/socket/dcsctp_socket.cc @@ -1021,12 +1021,6 @@ void DcSctpSocket::OnSentPacket(rtc::ArrayView packet, DebugPrintOutgoing(packet); } - // The heartbeat interval timer is restarted for every sent packet, to - // fire when the outgoing channel is inactive. - if (tcb_ != nullptr) { - tcb_->heartbeat_handler().RestartTimer(); - } - ++metrics_.tx_packets_count; } } diff --git a/net/dcsctp/socket/dcsctp_socket_test.cc b/net/dcsctp/socket/dcsctp_socket_test.cc index 2d392d62f1..d27b260753 100644 --- a/net/dcsctp/socket/dcsctp_socket_test.cc +++ b/net/dcsctp/socket/dcsctp_socket_test.cc @@ -910,6 +910,37 @@ TEST_P(DcSctpSocketParametrizedTest, ExpectHeartbeatToBeSent) { MaybeHandoverSocketAndSendMessage(a, std::move(z)); } +TEST(DcSctpSocketParametrizedTest, BothSidesSendHeartbeats) { + // On an idle connection, both sides send heartbeats, and both sides acks. + + // Make them have slightly different heartbeat intervals, to validate that + // sending an ack by Z doesn't restart its heartbeat timer. + DcSctpOptions options_a = {.heartbeat_interval = DurationMs(1000)}; + SocketUnderTest a("A", options_a); + + DcSctpOptions options_z = {.heartbeat_interval = DurationMs(1100)}; + SocketUnderTest z("Z", options_z); + + ConnectSockets(a, z); + + AdvanceTime(a, z, TimeDelta::Millis(1000)); + + std::vector packet_a = a.cb.ConsumeSentPacket(); + EXPECT_THAT(packet_a, HasChunks(ElementsAre(IsHeartbeatRequest(_)))); + // Z receives heartbeat, sends ACK. + z.socket.ReceivePacket(packet_a); + a.socket.ReceivePacket(z.cb.ConsumeSentPacket()); + + // A little while later, Z should send heartbeats to A. + AdvanceTime(a, z, TimeDelta::Millis(100)); + + std::vector packet_z = z.cb.ConsumeSentPacket(); + EXPECT_THAT(packet_z, HasChunks(ElementsAre(IsHeartbeatRequest(_)))); + // A receives heartbeat, sends ACK. + a.socket.ReceivePacket(packet_z); + z.socket.ReceivePacket(a.cb.ConsumeSentPacket()); +} + TEST_P(DcSctpSocketParametrizedTest, CloseConnectionAfterTooManyLostHeartbeats) { SocketUnderTest a("A"); diff --git a/net/dcsctp/socket/transmission_control_block.cc b/net/dcsctp/socket/transmission_control_block.cc index 34223411cb..e179a8e4ae 100644 --- a/net/dcsctp/socket/transmission_control_block.cc +++ b/net/dcsctp/socket/transmission_control_block.cc @@ -244,6 +244,13 @@ void TransmissionControlBlock::SendBufferedPackets(SctpPacket::Builder& builder, auto chunks = retransmission_queue_.GetChunksToSend(now, builder.bytes_remaining()); + + if (!chunks.empty()) { + // https://datatracker.ietf.org/doc/html/rfc9260#section-8.3 + // Sending DATA means that the path is not idle - restart heartbeat timer. + heartbeat_handler_.RestartTimer(); + } + for (auto& [tsn, data] : chunks) { if (capabilities_.message_interleaving) { builder.Add(IDataChunk(tsn, std::move(data), false));