From 07f2980f437dfa7cf268830ff44e91e35188612c Mon Sep 17 00:00:00 2001 From: Hani Damlaj Date: Tue, 17 Sep 2024 00:11:53 -0700 Subject: [PATCH] fix onBidirectionalStreamsAvailable Summary: fixes the arg passed to the ::onBidirectionalStreamsAvailable callback #facebook: i accidentally introduced a bad operator precedence bug in a refactor; this fixes that and the unit test luckily HQSession doesn't consume the argument passed to the callback Reviewed By: knekritz Differential Revision: D62765762 fbshipit-source-id: 7ae206d5036fc744485736f92fd366ce99103d58 --- quic/api/QuicTransportBase.cpp | 10 ++++++---- quic/api/test/QuicTransportBaseTest.cpp | 10 ++++++---- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/quic/api/QuicTransportBase.cpp b/quic/api/QuicTransportBase.cpp index 9bd57ad85..413741997 100644 --- a/quic/api/QuicTransportBase.cpp +++ b/quic/api/QuicTransportBase.cpp @@ -1064,15 +1064,17 @@ void QuicTransportBase::invokeStreamsAvailableCallbacks() { if (conn_->streamManager->consumeMaxLocalBidirectionalStreamIdIncreased()) { // check in case new streams were created in preceding callbacks // and max is already reached - if (auto numStreams = getNumOpenableBidirectionalStreams() > 0) { - connCallback_->onBidirectionalStreamsAvailable(numStreams); + auto numOpenableStreams = getNumOpenableBidirectionalStreams(); + if (numOpenableStreams > 0) { + connCallback_->onBidirectionalStreamsAvailable(numOpenableStreams); } } if (conn_->streamManager->consumeMaxLocalUnidirectionalStreamIdIncreased()) { // check in case new streams were created in preceding callbacks // and max is already reached - if (auto numStreams = getNumOpenableUnidirectionalStreams() > 0) { - connCallback_->onUnidirectionalStreamsAvailable(numStreams); + auto numOpenableStreams = getNumOpenableUnidirectionalStreams(); + if (numOpenableStreams > 0) { + connCallback_->onUnidirectionalStreamsAvailable(numOpenableStreams); } } } diff --git a/quic/api/test/QuicTransportBaseTest.cpp b/quic/api/test/QuicTransportBaseTest.cpp index 50a3b7a5a..20d34366d 100644 --- a/quic/api/test/QuicTransportBaseTest.cpp +++ b/quic/api/test/QuicTransportBaseTest.cpp @@ -1636,13 +1636,15 @@ TEST_P(QuicTransportImplTestBase, onBidiStreamsAvailableCallback) { EXPECT_CALL(connCallback, onBidirectionalStreamsAvailable(_)) .WillOnce(Invoke([](uint64_t numAvailableStreams) { - EXPECT_EQ(numAvailableStreams, 1); + EXPECT_EQ(numAvailableStreams, 10); })); - transport->addMaxStreamsFrame(MaxStreamsFrame(1, /*isBidirectionalIn=*/true)); - EXPECT_EQ(transport->getNumOpenableBidirectionalStreams(), 1); + transport->addMaxStreamsFrame( + MaxStreamsFrame(10, /*isBidirectionalIn=*/true)); + EXPECT_EQ(transport->getNumOpenableBidirectionalStreams(), 10); // same value max streams frame doesn't trigger callback - transport->addMaxStreamsFrame(MaxStreamsFrame(1, /*isBidirectionalIn=*/true)); + transport->addMaxStreamsFrame( + MaxStreamsFrame(10, /*isBidirectionalIn=*/true)); } TEST_P(QuicTransportImplTestBase, onBidiStreamsAvailableCallbackAfterExausted) {