diff --git a/quic/api/QuicTransportBase.cpp b/quic/api/QuicTransportBase.cpp index 27b6879f6..83584dd42 100644 --- a/quic/api/QuicTransportBase.cpp +++ b/quic/api/QuicTransportBase.cpp @@ -387,11 +387,6 @@ void QuicTransportBase::closeImpl( // Clear out all the streams, we don't need them any more. When the peer // receives the conn close they will implicitly reset all the streams. - QUIC_STATS_FOR_EACH( - conn_->streamManager->streams().cbegin(), - conn_->streamManager->streams().cend(), - conn_->statsCallback, - onQuicStreamClosed); conn_->streamManager->clearOpenStreams(); // Clear out all the buffered datagrams diff --git a/quic/api/test/QuicTransportBaseTest.cpp b/quic/api/test/QuicTransportBaseTest.cpp index 48dcd75ef..3c03f9729 100644 --- a/quic/api/test/QuicTransportBaseTest.cpp +++ b/quic/api/test/QuicTransportBaseTest.cpp @@ -22,6 +22,7 @@ #include #include #include +#include #include #include @@ -4289,6 +4290,26 @@ TEST_P(QuicTransportImplTestBase, BackgroundModeChangeWithStreamChanges) { manager.removeClosedStream(stream2Id); } +class QuicTransportImplTestCounters : public QuicTransportImplTest {}; + +TEST_F(QuicTransportImplTestCounters, TransportResetClosesStreams) { + MockQuicStats quicStats; + auto transportSettings = transport->getTransportSettings(); + auto& conn = transport->getConnectionState(); + conn.statsCallback = &quicStats; + + EXPECT_CALL(quicStats, onNewQuicStream()).Times(2); + EXPECT_CALL(quicStats, onQuicStreamClosed()).Times(2); + + auto stream1 = transport->createBidirectionalStream().value(); + auto stream2 = transport->createBidirectionalStream().value(); + + EXPECT_EQ(stream1, 1); + EXPECT_EQ(stream2, 5); + EXPECT_EQ(conn.streamManager->streamCount(), 2); + transport.reset(); +} + class QuicTransportImplTestWithGroups : public QuicTransportImplTestBase {}; INSTANTIATE_TEST_SUITE_P( diff --git a/quic/api/test/QuicTransportFunctionsTest.cpp b/quic/api/test/QuicTransportFunctionsTest.cpp index 6d4af3dfa..5a2a78274 100644 --- a/quic/api/test/QuicTransportFunctionsTest.cpp +++ b/quic/api/test/QuicTransportFunctionsTest.cpp @@ -4757,5 +4757,45 @@ TEST_F( expectedSignal.maybeBytesToSend, congestionControlWritableBytes(*conn)); } +TEST_F( + QuicTransportFunctionsTest, + onQuicStreamClosedNotCalledOnStreamClearing) { + { + auto conn = createConn(); + EXPECT_CALL(*quicStats_, onNewQuicStream()).Times(1); + EXPECT_CALL(*quicStats_, onQuicStreamClosed()).Times(1); + auto stream = conn->streamManager->createNextBidirectionalStream().value(); + EXPECT_EQ(stream->id, 1); + EXPECT_EQ(conn->streamManager->streamCount(), 1); + conn->streamManager->clearOpenStreams(); + EXPECT_EQ(conn->streamManager->streamCount(), 0); + } +} + +TEST_F(QuicTransportFunctionsTest, onQuicStreamClosedCalledOnConnClosure) { + { + auto conn = createConn(); + EXPECT_CALL(*quicStats_, onNewQuicStream()).Times(1); + EXPECT_CALL(*quicStats_, onQuicStreamClosed()).Times(1); + auto stream = conn->streamManager->createNextBidirectionalStream().value(); + EXPECT_EQ(stream->id, 1); + EXPECT_EQ(conn->streamManager->streamCount(), 1); + conn.reset(); + } +} + +TEST_F(QuicTransportFunctionsTest, onQuicStreamClosed) { + auto conn = createConn(); + EXPECT_CALL(*quicStats_, onNewQuicStream()).Times(1); + EXPECT_CALL(*quicStats_, onQuicStreamClosed()).Times(1); + auto stream = conn->streamManager->createNextBidirectionalStream().value(); + EXPECT_EQ(stream->id, 1); + EXPECT_EQ(conn->streamManager->streamCount(), 1); + stream->sendState = StreamSendState::Closed; + stream->recvState = StreamRecvState::Closed; + conn->streamManager->removeClosedStream(stream->id); + EXPECT_EQ(conn->streamManager->streamCount(), 0); +} + } // namespace test } // namespace quic diff --git a/quic/server/state/ServerStateMachine.h b/quic/server/state/ServerStateMachine.h index 37dcb5921..bf96e7cd1 100644 --- a/quic/server/state/ServerStateMachine.h +++ b/quic/server/state/ServerStateMachine.h @@ -89,7 +89,9 @@ struct MaxPacingRateKnobState { }; struct QuicServerConnectionState : public QuicConnectionStateBase { - ~QuicServerConnectionState() override = default; + ~QuicServerConnectionState() override { + streamManager->clearOpenStreams(); + } ServerState state; diff --git a/quic/state/QuicStreamManager.cpp b/quic/state/QuicStreamManager.cpp index 5f37760b3..c67f68414 100644 --- a/quic/state/QuicStreamManager.cpp +++ b/quic/state/QuicStreamManager.cpp @@ -7,6 +7,7 @@ #include #include +#include #include namespace quic { @@ -779,4 +780,22 @@ void QuicStreamManager::addToStreamPriorityMap( notifyStreamPriorityChanges(); } +void QuicStreamManager::clearOpenStreams() { + QUIC_STATS_FOR_EACH( + streams().cbegin(), + streams().cend(), + conn_.statsCallback, + onQuicStreamClosed); + + openBidirectionalLocalStreams_.clear(); + openUnidirectionalLocalStreams_.clear(); + openBidirectionalPeerStreams_.clear(); + openUnidirectionalPeerStreams_.clear(); + openBidirectionalLocalStreamGroups_.clear(); + openUnidirectionalLocalStreamGroups_.clear(); + peerUnidirectionalStreamGroupsSeen_.clear(); + peerBidirectionalStreamGroupsSeen_.clear(); + streams_.clear(); +} + } // namespace quic diff --git a/quic/state/QuicStreamManager.h b/quic/state/QuicStreamManager.h index b8bde91b5..297bc13c5 100644 --- a/quic/state/QuicStreamManager.h +++ b/quic/state/QuicStreamManager.h @@ -406,17 +406,7 @@ class QuicStreamManager { /* * Clear all the currently open streams. */ - void clearOpenStreams() { - openBidirectionalLocalStreams_.clear(); - openUnidirectionalLocalStreams_.clear(); - openBidirectionalPeerStreams_.clear(); - openUnidirectionalPeerStreams_.clear(); - openBidirectionalLocalStreamGroups_.clear(); - openUnidirectionalLocalStreamGroups_.clear(); - peerUnidirectionalStreamGroupsSeen_.clear(); - peerBidirectionalStreamGroupsSeen_.clear(); - streams_.clear(); - } + void clearOpenStreams(); /* * Return a const reference to the underlying container holding the stream