Skip to content

Commit

Permalink
Remove superfluous looping through all resets
Browse files Browse the repository at this point in the history
Summary: There's no need to loop through all the streams if we're just resetting one.

Reviewed By: hanidamlaj

Differential Revision: D67304631

fbshipit-source-id: 4817459ca7d1c1fd906b7640a0089c1c52e6e485
  • Loading branch information
Aman Sharma authored and facebook-github-bot committed Dec 18, 2024
1 parent 4d0ba52 commit 683b982
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 6 deletions.
7 changes: 1 addition & 6 deletions quic/api/QuicTransportBaseLite.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -430,12 +430,7 @@ folly::Expected<folly::Unit, LocalErrorCode> QuicTransportBaseLite::resetStream(
// Invoke state machine
sendRstSMHandler(*stream, errorCode);

for (auto pendingResetIt = conn_->pendingEvents.resets.begin();
closeState_ == CloseState::OPEN &&
pendingResetIt != conn_->pendingEvents.resets.end();
pendingResetIt++) {
cancelByteEventCallbacksForStream(pendingResetIt->first);
}
cancelByteEventCallbacksForStream(id);
pendingWriteCallbacks_.erase(id);
QUIC_STATS(conn_->statsCallback, onQuicStreamReset, errorCode);
} catch (const QuicTransportException& ex) {
Expand Down
23 changes: 23 additions & 0 deletions quic/api/test/QuicTransportBaseTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3091,6 +3091,29 @@ TEST_P(QuicTransportImplTestBase, TestGracefulCloseWithNoActiveStream) {
.hasError());
}

TEST_P(QuicTransportImplTestBase, TestResetRemovesDeliveryCb) {
auto stream1 = transport->createBidirectionalStream().value();
auto stream2 = transport->createBidirectionalStream().value();
NiceMock<MockDeliveryCallback> deliveryCb1;
NiceMock<MockDeliveryCallback> deliveryCb2;
EXPECT_CALL(*socketPtr, write(_, _, _))
.WillRepeatedly(SetErrnoAndReturn(EAGAIN, -1));
transport->writeChain(stream1, IOBuf::copyBuffer("hello"), true, nullptr);
transport->writeChain(stream2, IOBuf::copyBuffer("hello"), true, nullptr);
EXPECT_FALSE(
transport->registerDeliveryCallback(stream1, 2, &deliveryCb1).hasError());
EXPECT_FALSE(
transport->registerDeliveryCallback(stream2, 2, &deliveryCb2).hasError());
EXPECT_EQ(transport->getNumByteEventCallbacksForStream(stream1), 1);
EXPECT_EQ(transport->getNumByteEventCallbacksForStream(stream2), 1);
EXPECT_FALSE(
transport->resetStream(stream1, GenericApplicationErrorCode::UNKNOWN)
.hasError());
EXPECT_EQ(transport->getNumByteEventCallbacksForStream(stream1), 0);
EXPECT_EQ(transport->getNumByteEventCallbacksForStream(stream2), 1);
transport->close(none);
}

TEST_P(QuicTransportImplTestBase, TestImmediateClose) {
auto stream = transport->createBidirectionalStream().value();
auto stream2 = transport->createBidirectionalStream().value();
Expand Down

0 comments on commit 683b982

Please sign in to comment.