lib/mvfst
1
0
Fork 0
mirror of https://github.com/facebookincubator/mvfst.git synced 2025-08-08 09:42:06 +03:00
Code Activity

Don't remove stream from readable if the read callback isn't set.

Browse Source 
Summary: This probably never made much sense as a policy. The application can reasonably expect to be able to set a read callback after the stream first becomes readable.

Reviewed By: afrind

Differential Revision: D29563483

fbshipit-source-id: 215a6aed130108248a0b4cbc36c02c8da70bf721
This commit is contained in:
Matt Joras
2021-07-08 10:29:39 -07:00
committed by Facebook GitHub Bot
parent 3c1c51f5ef
commit 07af79fc65
2 changed files with 47 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
quic/api/QuicTransportBase.cpp
View File

@@ -798,7 +798,7 @@ void QuicTransportBase::invokeReadDataAndCallbacks() {
for (StreamId streamId : readableStreamsCopy) { for (StreamId streamId : readableStreamsCopy) {
auto callback = self->readCallbacks_.find(streamId); auto callback = self->readCallbacks_.find(streamId);
if (callback == self->readCallbacks_.end()) { if (callback == self->readCallbacks_.end()) {
self->conn_->streamManager->readableStreams().erase(streamId); // Stream doesn't have a read callback set, skip it.
continue; continue;
} }
auto readCb = callback->second.readCb; auto readCb = callback->second.readCb;

46
quic/api/test/QuicTransportBaseTest.cpp
View File

@@ -652,6 +652,52 @@ TEST_F(QuicTransportImplTest, ReadCallbackDataAvailable) {
transport.reset(); transport.reset();
} }
TEST_F(QuicTransportImplTest, ReadCallbackDataAvailableNoReap) {
auto stream1 = transport->createBidirectionalStream().value();
auto stream2 = transport->createBidirectionalStream().value();
StreamId stream3 = 0x6;
NiceMock<MockReadCallback> readCb1;
NiceMock<MockReadCallback> readCb2;
NiceMock<MockReadCallback> readCb3;
transport->setReadCallback(stream1, &readCb1);
transport->setReadCallback(stream2, &readCb2);
transport->addDataToStream(
stream1, StreamBuffer(folly::IOBuf::copyBuffer("actual stream data"), 0));
transport->addDataToStream(
stream2,
StreamBuffer(folly::IOBuf::copyBuffer("actual stream data"), 10));
transport->addDataToStream(
stream3, StreamBuffer(folly::IOBuf::copyBuffer("actual stream data"), 0));
EXPECT_CALL(readCb1, readAvailable(stream1));
transport->driveReadCallbacks();
transport->setReadCallback(stream3, &readCb3);
transport->addDataToStream(
stream2, StreamBuffer(folly::IOBuf::copyBuffer("actual stream data"), 0));
EXPECT_CALL(readCb1, readAvailable(stream1));
EXPECT_CALL(readCb2, readAvailable(stream2));
EXPECT_CALL(readCb3, readAvailable(stream3));
transport->driveReadCallbacks();
EXPECT_CALL(readCb1, readAvailable(stream1));
EXPECT_CALL(readCb2, readAvailable(stream2));
EXPECT_CALL(readCb3, readAvailable(stream3));
transport->driveReadCallbacks();
EXPECT_CALL(readCb2, readAvailable(stream2));
EXPECT_CALL(readCb3, readAvailable(stream3));
transport->setReadCallback(stream1, nullptr);
transport->driveReadCallbacks();
transport.reset();
}
TEST_F(QuicTransportImplTest, ReadCallbackDataAvailableOrdered) { TEST_F(QuicTransportImplTest, ReadCallbackDataAvailableOrdered) {
auto transportSettings = transport->getTransportSettings(); auto transportSettings = transport->getTransportSettings();
transportSettings.orderedReadCallbacks = true; transportSettings.orderedReadCallbacks = true;