diff --git a/quic/api/QuicTransportBase.h b/quic/api/QuicTransportBase.h index 8c945d5b0..5dbb4e75b 100644 --- a/quic/api/QuicTransportBase.h +++ b/quic/api/QuicTransportBase.h @@ -34,7 +34,7 @@ namespace quic { * of the object that holds it to send graceful close messages to the peer. */ class QuicTransportBase : public QuicSocket, - public QuicTransportBaseLite, + virtual public QuicTransportBaseLite, QuicStreamPrioritiesObserver { public: QuicTransportBase( diff --git a/quic/api/QuicTransportBaseLite.h b/quic/api/QuicTransportBaseLite.h index 084122181..be1934eb4 100644 --- a/quic/api/QuicTransportBaseLite.h +++ b/quic/api/QuicTransportBaseLite.h @@ -21,7 +21,7 @@ class QuicTransportBaseLite : virtual public QuicSocketLite, QuicTransportBaseLite( std::shared_ptr evb, std::unique_ptr socket, - bool useConnectionEndWithErrorCallback); + bool useConnectionEndWithErrorCallback = false); /** * Invoked when we have to write some data to the wire. diff --git a/quic/api/test/Mocks.h b/quic/api/test/Mocks.h index ad3ad8a33..fe362ea29 100644 --- a/quic/api/test/Mocks.h +++ b/quic/api/test/Mocks.h @@ -212,9 +212,12 @@ class MockQuicTransport : public QuicServerTransport { ConnectionSetupCallback* connSetupCb, ConnectionCallback* connCb, std::shared_ptr ctx) - : QuicServerTransport( + : QuicTransportBaseLite(evb, std::move(sock)), + QuicServerTransport( std::move(evb), - std::move(sock), + nullptr /* Initialized through the QuicTransportBaseLite constructor + */ + , connSetupCb, connCb, ctx) {} diff --git a/quic/api/test/QuicTransportBaseTest.cpp b/quic/api/test/QuicTransportBaseTest.cpp index 773ae1a75..048040c47 100644 --- a/quic/api/test/QuicTransportBaseTest.cpp +++ b/quic/api/test/QuicTransportBaseTest.cpp @@ -232,8 +232,7 @@ class TestQuicTransport std::unique_ptr socket, ConnectionSetupCallback* connSetupCb, ConnectionCallback* connCb) - : QuicTransportBase(std::move(evb), std::move(socket)), - observerContainer_(std::make_shared(this)) { + : QuicTransportBaseLite(evb, std::move(socket)), QuicTransportBase(evb, nullptr /* Initialized through the QuicTransportBaseLite constructor */), observerContainer_(std::make_shared(this)) { auto conn = std::make_unique( FizzServerQuicHandshakeContext::Builder().build()); conn->clientConnectionId = ConnectionId({10, 9, 8, 7}); diff --git a/quic/api/test/TestQuicTransport.h b/quic/api/test/TestQuicTransport.h index fba9d16fd..6aafd731d 100644 --- a/quic/api/test/TestQuicTransport.h +++ b/quic/api/test/TestQuicTransport.h @@ -24,8 +24,7 @@ class TestQuicTransport std::unique_ptr socket, ConnectionSetupCallback* connSetupCb, ConnectionCallback* connCb) - : QuicTransportBase(std::move(evb), std::move(socket)), - observerContainer_(std::make_shared(this)) { + : QuicTransportBaseLite(evb, std::move(socket)), QuicTransportBase(evb, nullptr /* Initialized through the QuicTransportBaseLite constructor */), observerContainer_(std::make_shared(this)) { conn_.reset(new QuicServerConnectionState( FizzServerQuicHandshakeContext::Builder().build())); conn_->clientConnectionId = ConnectionId({9, 8, 7, 6}); diff --git a/quic/client/BUCK b/quic/client/BUCK index 379484843..86f2e3fab 100644 --- a/quic/client/BUCK +++ b/quic/client/BUCK @@ -15,6 +15,7 @@ mvfst_cpp_library( "//folly/portability:sockets", "//quic:constants", "//quic/api:loop_detector_callback", + "//quic/api:transport_helpers", "//quic/flowcontrol:flow_control", "//quic/handshake:handshake", "//quic/happyeyeballs:happyeyeballs", diff --git a/quic/client/QuicClientTransport.cpp b/quic/client/QuicClientTransport.cpp index 3e6c519f6..7857f051b 100644 --- a/quic/client/QuicClientTransport.cpp +++ b/quic/client/QuicClientTransport.cpp @@ -57,10 +57,16 @@ QuicClientTransport::QuicClientTransport( std::shared_ptr handshakeFactory, size_t connectionIdSize, bool useConnectionEndWithErrorCallback) - : QuicTransportBase( - std::move(evb), + : QuicTransportBaseLite( + evb, std::move(socket), useConnectionEndWithErrorCallback), + QuicTransportBase( + evb, + nullptr /* Initialized through the QuicTransportBaseLite constructor + */ + , + useConnectionEndWithErrorCallback), happyEyeballsConnAttemptDelayTimeout_(this), wrappedObserverContainer_(this) { DCHECK(handshakeFactory); @@ -1056,10 +1062,9 @@ void QuicClientTransport::startCryptoHandshake() { writeSocketData(); if (!transportReadyNotified_ && clientConn_->zeroRttWriteCipher) { transportReadyNotified_ = true; - runOnEvbAsync([](auto self) { - auto clientPtr = static_cast(self.get()); - if (clientPtr->connSetupCallback_) { - clientPtr->connSetupCallback_->onTransportReady(); + runOnEvbAsync([this](auto) { + if (connSetupCallback_) { + connSetupCallback_->onTransportReady(); } }); } @@ -1114,12 +1119,11 @@ void QuicClientTransport::errMessage( auto errStr = folly::errnoStr(serr->ee_errno); if (!happyEyeballsState.shouldWriteToFirstSocket && !happyEyeballsState.shouldWriteToSecondSocket) { - runOnEvbAsync([errString = std::move(errStr)](auto self) mutable { + runOnEvbAsync([errString = std::move(errStr), this](auto) mutable { auto quicError = QuicError( QuicErrorCode(LocalErrorCode::CONNECT_FAILED), std::move(errString)); - auto clientPtr = static_cast(self.get()); - clientPtr->closeImpl(std::move(quicError), false, false); + closeImpl(std::move(quicError), false, false); }); } } @@ -1132,9 +1136,8 @@ void QuicClientTransport::onReadError( // closeNow will skip draining the socket. onReadError doesn't gets // triggered by retriable errors. If we are here, there is no point of // draining the socket. - runOnEvbAsync([ex](auto self) { - auto clientPtr = static_cast(self.get()); - clientPtr->closeNow(QuicError( + runOnEvbAsync([ex, this](auto) { + closeNow(QuicError( QuicErrorCode(LocalErrorCode::CONNECTION_ABANDONED), std::string(ex.what()))); }); @@ -1755,22 +1758,19 @@ void QuicClientTransport::start( adjustGROBuffers(); startCryptoHandshake(); } catch (const QuicTransportException& ex) { - runOnEvbAsync([ex](auto self) { - auto clientPtr = static_cast(self.get()); - clientPtr->closeImpl( + runOnEvbAsync([ex, this](auto) { + closeImpl( QuicError(QuicErrorCode(ex.errorCode()), std::string(ex.what()))); }); } catch (const QuicInternalException& ex) { - runOnEvbAsync([ex](auto self) { - auto clientPtr = static_cast(self.get()); - clientPtr->closeImpl( + runOnEvbAsync([ex, this](auto) { + closeImpl( QuicError(QuicErrorCode(ex.errorCode()), std::string(ex.what()))); }); } catch (const std::exception& ex) { LOG(ERROR) << "Connect failed " << ex.what(); - runOnEvbAsync([ex](auto self) { - auto clientPtr = static_cast(self.get()); - clientPtr->closeImpl(QuicError( + runOnEvbAsync([ex, this](auto) { + closeImpl(QuicError( QuicErrorCode(TransportErrorCode::INTERNAL_ERROR), std::string(ex.what()))); }); diff --git a/quic/client/test/BUCK b/quic/client/test/BUCK index 445470821..589ea8dfa 100644 --- a/quic/client/test/BUCK +++ b/quic/client/test/BUCK @@ -29,7 +29,7 @@ cpp_unittest( supports_static_listing = False, deps = [ ":mocks", - "//quic/api:transport", + "//quic/api:transport_helpers", "//quic/api/test:mocks", "//quic/client:cached_server_tp", "//quic/client:state_and_handshake", diff --git a/quic/client/test/Mocks.h b/quic/client/test/Mocks.h index 247623de0..4642e96b5 100644 --- a/quic/client/test/Mocks.h +++ b/quic/client/test/Mocks.h @@ -106,9 +106,12 @@ class MockQuicClientTransport : public quic::QuicClientTransport { std::shared_ptr evb, std::unique_ptr socket, std::shared_ptr handshakeFactory) - : QuicClientTransport( + : QuicTransportBaseLite(evb, std::move(socket)), + QuicClientTransport( evb, - std::move(socket), + nullptr /* Initialized through the QuicTransportBaseLite constructor + */ + , std::move(handshakeFactory)), testType_(testType) {} diff --git a/quic/client/test/QuicClientTransportMock.h b/quic/client/test/QuicClientTransportMock.h index b3c901112..17d68a347 100644 --- a/quic/client/test/QuicClientTransportMock.h +++ b/quic/client/test/QuicClientTransportMock.h @@ -18,8 +18,9 @@ class QuicClientTransportMock : public QuicClientTransport { std::shared_ptr evb, std::unique_ptr socket, std::shared_ptr handshakeFactory) - : QuicClientTransport( - std::move(evb), + : QuicTransportBaseLite(evb, std::move(socket)), + QuicClientTransport( + evb, std::move(socket), std::move(handshakeFactory)) {} diff --git a/quic/client/test/QuicClientTransportTest.cpp b/quic/client/test/QuicClientTransportTest.cpp index f64e20de1..0fc810daf 100644 --- a/quic/client/test/QuicClientTransportTest.cpp +++ b/quic/client/test/QuicClientTransportTest.cpp @@ -22,7 +22,13 @@ class QuicClientTransportMock : public QuicClientTransport { std::shared_ptr evb, std::unique_ptr socket, std::shared_ptr handshakeFactory) - : QuicClientTransport(evb, std::move(socket), handshakeFactory) {} + : QuicTransportBaseLite(evb, std::move(socket)), + QuicClientTransport( + evb, + nullptr /* Initialized through the QuicTransportBaseLite constructor + */ + , + handshakeFactory) {} void readWithRecvmsg( QuicAsyncUDPSocket& sock, diff --git a/quic/fizz/client/test/QuicClientTransportTestUtil.h b/quic/fizz/client/test/QuicClientTransportTestUtil.h index 9127f3c35..9716e9180 100644 --- a/quic/fizz/client/test/QuicClientTransportTestUtil.h +++ b/quic/fizz/client/test/QuicClientTransportTestUtil.h @@ -49,9 +49,15 @@ class TestingQuicClientTransport : public QuicClientTransport { std::shared_ptr handshakeFactory, size_t connIdSize = kDefaultConnectionIdSize, bool useConnectionEndWithErrorCallback = false) - : QuicClientTransport( + : QuicTransportBaseLite( evb, std::move(socket), + useConnectionEndWithErrorCallback), + QuicClientTransport( + evb, + nullptr /* Initialized through the QuicTransportBaseLite constructor + */ + , std::move(handshakeFactory), connIdSize, useConnectionEndWithErrorCallback) {} diff --git a/quic/server/BUCK b/quic/server/BUCK index db0a4efc7..413e5f83f 100644 --- a/quic/server/BUCK +++ b/quic/server/BUCK @@ -71,6 +71,7 @@ mvfst_cpp_library( "//folly/io/async:scoped_event_base_thread", "//quic:constants", "//quic/api:transport", + "//quic/api:transport_helpers", "//quic/codec:types", "//quic/common:buf_accessor", "//quic/common:transport_knobs", diff --git a/quic/server/QuicServerTransport.cpp b/quic/server/QuicServerTransport.cpp index fdcd49b85..1a15a98d3 100644 --- a/quic/server/QuicServerTransport.cpp +++ b/quic/server/QuicServerTransport.cpp @@ -51,10 +51,16 @@ QuicServerTransport::QuicServerTransport( std::shared_ptr ctx, std::unique_ptr cryptoFactory, bool useConnectionEndWithErrorCallback) - : QuicTransportBase( - std::move(evb), + : QuicTransportBaseLite( + evb, std::move(sock), useConnectionEndWithErrorCallback), + QuicTransportBase( + std::move(evb), + nullptr /* Initialized through the QuicTransportBaseLite constructor + */ + , + useConnectionEndWithErrorCallback), ctx_(std::move(ctx)), wrappedObserverContainer_(this) { auto tempConn = std::make_unique( @@ -476,13 +482,12 @@ void QuicServerTransport::processPendingData(bool async) { VLOG_IF(10, !pendingData->empty()) << "Processing pending data size=" << pendingData->size() << " " << *this; - auto func = [pendingData = std::move(pendingData)](auto self) { - auto serverPtr = static_cast(self.get()); + auto func = [pendingData = std::move(pendingData), this](auto) { for (auto& pendingPacket : *pendingData) { - serverPtr->onNetworkData( + onNetworkData( pendingPacket.peer, NetworkData(std::move(pendingPacket.udpPacket))); - if (serverPtr->closeState_ == CloseState::CLOSED) { + if (closeState_ == CloseState::CLOSED) { // The pending data could potentially contain a connection close, or // the app could have triggered a connection close with an error. It // is not useful to continue the handshake. diff --git a/quic/server/test/BUCK b/quic/server/test/BUCK index 235116d66..0f2671ec7 100644 --- a/quic/server/test/BUCK +++ b/quic/server/test/BUCK @@ -62,7 +62,7 @@ mvfst_cpp_library( "fbsource//third-party/googletest:gmock", "fbsource//third-party/googletest:gtest", ":mocks", - "//quic/api:transport", + "//quic/api:transport_helpers", "//quic/api/test:mocks", "//quic/codec:types", "//quic/common:transport_knobs", diff --git a/quic/server/test/Mocks.h b/quic/server/test/Mocks.h index 299bb662b..7b01fbc95 100644 --- a/quic/server/test/Mocks.h +++ b/quic/server/test/Mocks.h @@ -150,9 +150,12 @@ class MockQuicServerTransport : public QuicServerTransport { MockQuicServerTransport( std::shared_ptr evb, std::unique_ptr sock) - : QuicServerTransport( - std::move(evb), - std::move(sock), + : QuicTransportBaseLite(evb, std::move(sock)), + QuicServerTransport( + evb, + nullptr /* Initialized through the QuicTransportBaseLite constructor + */ + , nullptr, nullptr, nullptr) {} diff --git a/quic/server/test/QuicServerTransportTestUtil.h b/quic/server/test/QuicServerTransportTestUtil.h index 3b90f85da..a156955cf 100644 --- a/quic/server/test/QuicServerTransportTestUtil.h +++ b/quic/server/test/QuicServerTransportTestUtil.h @@ -36,9 +36,12 @@ class TestingQuicServerTransport : public QuicServerTransport { ConnectionSetupCallback* connSetupCb, ConnectionCallback* connCb, std::shared_ptr ctx) - : QuicServerTransport( - std::move(evb), - std::move(sock), + : QuicTransportBaseLite(evb, std::move(sock)), + QuicServerTransport( + evb, + nullptr /* Initialized through the QuicTransportBaseLite constructor + */ + , connSetupCb, connCb, std::move(ctx)) {}