lib/mvfst
1
0
Fork 0
mirror of https://github.com/facebookincubator/mvfst.git synced 2025-07-29 03:41:11 +03:00
Code Activity

Remove resets for stream upon closure

Browse Source 
Summary: With reliable resets, we can end up in a situation where we send two reliable resets, but only one has been egressed and ACKed before the stream closes. Therefore, we need to take care to ensure that `conn_.pendingEvents.resets` doesn't contain the `streamId` for the stream being removed.

Reviewed By: kvtsoy

Differential Revision: D67108782

fbshipit-source-id: 183512a579a6882edef15579a7427f0afccb253f
This commit is contained in:
Aman Sharma
2024-12-13 13:17:30 -08:00
committed by Facebook GitHub Bot
parent a74e85d49e
commit 9fe7f83eca
3 changed files with 22 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
quic/api/QuicPacketScheduler.cpp
View File

@ -648,6 +648,8 @@ bool RstStreamScheduler::writeRsts(PacketBuilderInterface& builder) {
for (const auto& resetStream : conn_.pendingEvents.resets) { for (const auto& resetStream : conn_.pendingEvents.resets) {
auto streamId = resetStream.first; auto streamId = resetStream.first;
QuicStreamState* streamState = conn_.streamManager->getStream(streamId); QuicStreamState* streamState = conn_.streamManager->getStream(streamId);
CHECK(streamState) << "Stream " << streamId
<< " not found when going through resets";
if (streamState->pendingWrites.empty() && if (streamState->pendingWrites.empty() &&
streamState->writeBufMeta.length == 0) { streamState->writeBufMeta.length == 0) {
// We only write a RESET_STREAM or RESET_STREAM_AT frame for a stream // We only write a RESET_STREAM or RESET_STREAM_AT frame for a stream

5
quic/state/QuicStreamManager.cpp
View File

@ -555,6 +555,11 @@ void QuicStreamManager::removeClosedStream(StreamId streamId) {
} }
VLOG(10) << "Removing closed stream=" << streamId; VLOG(10) << "Removing closed stream=" << streamId;
DCHECK(it->second.inTerminalStates()); DCHECK(it->second.inTerminalStates());
if (conn_.pendingEvents.resets.contains(streamId)) {
// This can happen when we send two reliable resets, one of which is
// egressed and ACKed.
conn_.pendingEvents.resets.erase(streamId);
}
if (conn_.transportSettings.unidirectionalStreamsReadCallbacksFirst && if (conn_.transportSettings.unidirectionalStreamsReadCallbacksFirst &&
isUnidirectionalStream(streamId)) { isUnidirectionalStream(streamId)) {
unidirectionalReadableStreams_.erase(streamId); unidirectionalReadableStreams_.erase(streamId);

15
quic/state/test/QuicStreamManagerTest.cpp
View File

@ -826,6 +826,21 @@ TEST_P(QuicStreamManagerTest, StreamPriorityExcludesControl) {
manager.removeClosedStream(stream->id); manager.removeClosedStream(stream->id);
} }
TEST_P(QuicStreamManagerTest, RemoveResetsUponClosure) {
MockQuicStreamPrioritiesObserver mObserver;
auto& manager = *conn.streamManager;
auto stream = manager.createNextBidirectionalStream().value();
conn.pendingEvents.resets.emplace(
stream->id, RstStreamFrame(stream->id, 0, 0));
stream->sendState = StreamSendState::Closed;
stream->recvState = StreamRecvState::Closed;
EXPECT_TRUE(conn.pendingEvents.resets.contains(stream->id));
manager.removeClosedStream(stream->id);
EXPECT_FALSE(conn.pendingEvents.resets.contains(stream->id));
}
Buf createBuffer(uint32_t len) { Buf createBuffer(uint32_t len) {
auto buf = folly::IOBuf::create(len); auto buf = folly::IOBuf::create(len);
buf->append(len); buf->append(len);