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

QuicTransportBase: Allow notifyPendingWriteOnStream (#18)

Browse Source 
Summary:
When a write callback x is already defined for a stream
and notifyPendingWriteOnStream is called with the same callback x,
it is safe to not return an error.

Building on master is currently broken due to 2040a13e40 (diff-ddd35d436da215e41b89fd5afe4acebbR11), but this commit builds when rebased before that.

udippant
Pull Request resolved: https://github.com/facebookincubator/mvfst/pull/18

Reviewed By: yangchi

Differential Revision: D15562893

Pulled By: afrind

fbshipit-source-id: 44e62bb9bb0002508257acdc419f20bcb973ca64
This commit is contained in:
Akshay Narayan
2019-06-05 16:52:46 -07:00
committed by Facebook Github Bot
parent 0b677465d0
commit 7d58058b8f
4 changed files with 25 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
quic/QuicConstants.h
View File

@@ -159,6 +159,7 @@ enum class LocalErrorCode : uint32_t {
INVALID_OPERATION = 0x40000017,
STREAM_LIMIT_EXCEEDED = 0x40000018,
CONNECTION_ABANDONED = 0x40000019,
CALLBACK_ALREADY_INSTALLED = 0x4000001A,
};
using QuicErrorCode =

2
quic/QuicException.cpp
View File

@@ -84,6 +84,8 @@ std::string toString(LocalErrorCode code) {
return "New version negotiatied";
case LocalErrorCode::INVALID_WRITE_CALLBACK:
return "Invalid write callback";
case LocalErrorCode::CALLBACK_ALREADY_INSTALLED:
return "Callback already installed";
case LocalErrorCode::TLS_HANDSHAKE_FAILED:
return "TLS handshake failed";
case LocalErrorCode::APP_ERROR:

13
quic/api/QuicTransportBase.cpp
View File

@@ -1636,10 +1636,19 @@ QuicTransportBase::notifyPendingWriteOnStream(StreamId id, WriteCallback* wcb) {
if (!qStream->writable()) {
return folly::makeUnexpected(LocalErrorCode::STREAM_CLOSED);
}
if (wcb == nullptr) {
return folly::makeUnexpected(LocalErrorCode::INVALID_WRITE_CALLBACK);
}
// Add the callback to the pending write callbacks so that if we are closed
// while we are scheduled in the loop, the close will error out the callbacks.
if (!pendingWriteCallbacks_.emplace(id, wcb).second) {
return folly::makeUnexpected(LocalErrorCode::INVALID_WRITE_CALLBACK);
auto wcbEmplaceResult = pendingWriteCallbacks_.emplace(id, wcb);
if (!wcbEmplaceResult.second) {
if ((wcbEmplaceResult.first)->second != wcb) {
return folly::makeUnexpected(LocalErrorCode::INVALID_WRITE_CALLBACK);
} else {
return folly::makeUnexpected(LocalErrorCode::CALLBACK_ALREADY_INSTALLED);
}
}
runOnEvbAsync([id](auto self) {
auto wcbIt = self->pendingWriteCallbacks_.find(id);

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

@@ -1214,6 +1214,17 @@ TEST_P(QuicTransportImplTestClose, TestNotifyPendingConnWriteOnCloseWithError) {
evb->loopOnce();
}
TEST_F(QuicTransportImplTest, TestNotifyPendingWriteWithActiveCallback) {
auto stream = transport->createBidirectionalStream().value();
MockWriteCallback wcb;
EXPECT_CALL(wcb, onStreamWriteReady(stream, _));
auto ok1 = transport->notifyPendingWriteOnStream(stream, &wcb);
EXPECT_TRUE(ok1.hasValue());
auto ok2 = transport->notifyPendingWriteOnStream(stream, &wcb);
EXPECT_EQ(ok2.error(), quic::LocalErrorCode::CALLBACK_ALREADY_INSTALLED);
evb->loopOnce();
}
TEST_F(QuicTransportImplTest, TestNotifyPendingWriteOnCloseWithoutError) {
auto stream = transport->createBidirectionalStream().value();
MockWriteCallback wcb;