1
0
mirror of https://github.com/facebookincubator/mvfst.git synced 2025-08-06 22:22:38 +03:00

Do not checkForClosedStream from read.

Summary:
This is a pretty confusing API behavior. This means that when reading from stream A, stream B could potentially be removed if it doesn't have a read callback set. This effectively means you cannot safely call read from onNew*Stream.

There's no real reason the check for a closed stream here, as the streams will get cleaned up from the onNetworkData or read looper calls.

Reviewed By: yangchi

Differential Revision: D20848090

fbshipit-source-id: 38eba9e5a24b6ddf5bf05ac75787dda9c001c1c6
This commit is contained in:
Matt Joras
2020-04-06 10:12:37 -07:00
committed by Facebook GitHub Bot
parent a230720c3c
commit c826dd592b
2 changed files with 19 additions and 3 deletions

View File

@@ -829,6 +829,25 @@ TEST_F(QuicTransportImplTest, onNewBidirectionalStreamCallback) {
transport.reset();
}
TEST_F(QuicTransportImplTest, onNewStreamCallbackDoesNotRemove) {
auto readData = folly::IOBuf::copyBuffer("actual stream data");
StreamId uniStream1 = 2;
StreamId uniStream2 = uniStream1 + kStreamIncrement;
EXPECT_CALL(connCallback, onNewUnidirectionalStream(uniStream1))
.WillOnce(Invoke([&](StreamId id) {
ASSERT_FALSE(transport->read(id, 100).hasError());
}));
EXPECT_CALL(connCallback, onNewUnidirectionalStream(uniStream2))
.WillOnce(Invoke([&](StreamId id) {
ASSERT_FALSE(transport->read(id, 100).hasError());
}));
transport->addDataToStream(
uniStream1, StreamBuffer(readData->clone(), 0, true));
transport->addDataToStream(
uniStream2, StreamBuffer(readData->clone(), 0, true));
transport.reset();
}
TEST_F(QuicTransportImplTest, onNewBidirectionalStreamStreamOutOfOrder) {
InSequence dummy;
auto readData = folly::IOBuf::copyBuffer("actual stream data");