Make the QuicTransportBase -> QuicTransportBaseLite inheritance virtual

Summary: See title.

Reviewed By: mjoras

Differential Revision: D65688218

fbshipit-source-id: 1fcaf353f264191f411f0fd80be4cd7dbc3bb8ea

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(

quic/api/QuicTransportBaseLite.h

@@ -21,7 +21,7 @@ class QuicTransportBaseLite : virtual public QuicSocketLite,
   QuicTransportBaseLite(
       std::shared_ptr<QuicEventBase> evb,
       std::unique_ptr<QuicAsyncUDPSocket> socket,
-      bool useConnectionEndWithErrorCallback);
+      bool useConnectionEndWithErrorCallback = false);
 
   /**
    * Invoked when we have to write some data to the wire.

quic/api/test/Mocks.h

@@ -212,9 +212,12 @@ class MockQuicTransport : public QuicServerTransport {
       ConnectionSetupCallback* connSetupCb,
       ConnectionCallback* connCb,
       std::shared_ptr<const fizz::server::FizzServerContext> ctx)
-      : QuicServerTransport(
+      : QuicTransportBaseLite(evb, std::move(sock)),
+        QuicServerTransport(
             std::move(evb),
-            std::move(sock),
+            nullptr /* Initialized through the QuicTransportBaseLite constructor
+                     */
+            ,
             connSetupCb,
             connCb,
             ctx) {}

quic/api/test/QuicTransportBaseTest.cpp

@@ -232,8 +232,7 @@ class TestQuicTransport
       std::unique_ptr<QuicAsyncUDPSocket> socket,
       ConnectionSetupCallback* connSetupCb,
       ConnectionCallback* connCb)
-      : QuicTransportBase(std::move(evb), std::move(socket)),
+      : QuicTransportBaseLite(evb, std::move(socket)), QuicTransportBase(evb, nullptr /* Initialized through the QuicTransportBaseLite constructor */), observerContainer_(std::make_shared<SocketObserverContainer>(this)) {
-        observerContainer_(std::make_shared<SocketObserverContainer>(this)) {
     auto conn = std::make_unique<QuicServerConnectionState>(
         FizzServerQuicHandshakeContext::Builder().build());
     conn->clientConnectionId = ConnectionId({10, 9, 8, 7});

quic/api/test/TestQuicTransport.h

@@ -24,8 +24,7 @@ class TestQuicTransport
       std::unique_ptr<QuicAsyncUDPSocket> socket,
       ConnectionSetupCallback* connSetupCb,
       ConnectionCallback* connCb)
-      : QuicTransportBase(std::move(evb), std::move(socket)),
+      : QuicTransportBaseLite(evb, std::move(socket)), QuicTransportBase(evb, nullptr /* Initialized through the QuicTransportBaseLite constructor */), observerContainer_(std::make_shared<SocketObserverContainer>(this)) {
-        observerContainer_(std::make_shared<SocketObserverContainer>(this)) {
     conn_.reset(new QuicServerConnectionState(
         FizzServerQuicHandshakeContext::Builder().build()));
     conn_->clientConnectionId = ConnectionId({9, 8, 7, 6});

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",

quic/client/QuicClientTransport.cpp

@@ -57,10 +57,16 @@ QuicClientTransport::QuicClientTransport(
     std::shared_ptr<ClientHandshakeFactory> handshakeFactory,
     size_t connectionIdSize,
     bool useConnectionEndWithErrorCallback)
-    : QuicTransportBase(
+    : QuicTransportBaseLite(
-          std::move(evb),
+          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) {
+    runOnEvbAsync([this](auto) {
-      auto clientPtr = static_cast<QuicClientTransport*>(self.get());
+      if (connSetupCallback_) {
-      if (clientPtr->connSetupCallback_) {
+        connSetupCallback_->onTransportReady();
-        clientPtr->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<QuicClientTransport*>(self.get());
+        closeImpl(std::move(quicError), false, false);
-        clientPtr->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) {
+    runOnEvbAsync([ex, this](auto) {
-      auto clientPtr = static_cast<QuicClientTransport*>(self.get());
+      closeNow(QuicError(
-      clientPtr->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) {
+    runOnEvbAsync([ex, this](auto) {
-      auto clientPtr = static_cast<QuicClientTransport*>(self.get());
+      closeImpl(
-      clientPtr->closeImpl(
           QuicError(QuicErrorCode(ex.errorCode()), std::string(ex.what())));
     });
   } catch (const QuicInternalException& ex) {
-    runOnEvbAsync([ex](auto self) {
+    runOnEvbAsync([ex, this](auto) {
-      auto clientPtr = static_cast<QuicClientTransport*>(self.get());
+      closeImpl(
-      clientPtr->closeImpl(
           QuicError(QuicErrorCode(ex.errorCode()), std::string(ex.what())));
     });
   } catch (const std::exception& ex) {
     LOG(ERROR) << "Connect failed " << ex.what();
-    runOnEvbAsync([ex](auto self) {
+    runOnEvbAsync([ex, this](auto) {
-      auto clientPtr = static_cast<QuicClientTransport*>(self.get());
+      closeImpl(QuicError(
-      clientPtr->closeImpl(QuicError(
           QuicErrorCode(TransportErrorCode::INTERNAL_ERROR),
           std::string(ex.what())));
     });

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",

quic/client/test/Mocks.h

@@ -106,9 +106,12 @@ class MockQuicClientTransport : public quic::QuicClientTransport {
       std::shared_ptr<QuicEventBase> evb,
       std::unique_ptr<QuicAsyncUDPSocket> socket,
       std::shared_ptr<ClientHandshakeFactory> handshakeFactory)
-      : QuicClientTransport(
+      : QuicTransportBaseLite(evb, std::move(socket)),
+        QuicClientTransport(
             evb,
-            std::move(socket),
+            nullptr /* Initialized through the QuicTransportBaseLite constructor
+                     */
+            ,
             std::move(handshakeFactory)),
         testType_(testType) {}
 

quic/client/test/QuicClientTransportMock.h

@@ -18,8 +18,9 @@ class QuicClientTransportMock : public QuicClientTransport {
       std::shared_ptr<QuicEventBase> evb,
       std::unique_ptr<QuicAsyncUDPSocket> socket,
       std::shared_ptr<ClientHandshakeFactory> handshakeFactory)
-      : QuicClientTransport(
+      : QuicTransportBaseLite(evb, std::move(socket)),
-            std::move(evb),
+        QuicClientTransport(
+            evb,
             std::move(socket),
             std::move(handshakeFactory)) {}
 

quic/client/test/QuicClientTransportTest.cpp

@@ -22,7 +22,13 @@ class QuicClientTransportMock : public QuicClientTransport {
       std::shared_ptr<QuicEventBase> evb,
       std::unique_ptr<QuicAsyncUDPSocket> socket,
       std::shared_ptr<ClientHandshakeFactory> 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,

quic/fizz/client/test/QuicClientTransportTestUtil.h

@@ -49,9 +49,15 @@ class TestingQuicClientTransport : public QuicClientTransport {
       std::shared_ptr<ClientHandshakeFactory> 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) {}

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",

quic/server/QuicServerTransport.cpp

@@ -51,10 +51,16 @@ QuicServerTransport::QuicServerTransport(
     std::shared_ptr<const fizz::server::FizzServerContext> ctx,
     std::unique_ptr<CryptoFactory> cryptoFactory,
     bool useConnectionEndWithErrorCallback)
-    : QuicTransportBase(
+    : QuicTransportBaseLite(
-          std::move(evb),
+          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<QuicServerConnectionState>(
@@ -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 func = [pendingData = std::move(pendingData), this](auto) {
-      auto serverPtr = static_cast<QuicServerTransport*>(self.get());
       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.

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",

quic/server/test/Mocks.h

@@ -150,9 +150,12 @@ class MockQuicServerTransport : public QuicServerTransport {
   MockQuicServerTransport(
       std::shared_ptr<FollyQuicEventBase> evb,
       std::unique_ptr<FollyQuicAsyncUDPSocket> sock)
-      : QuicServerTransport(
+      : QuicTransportBaseLite(evb, std::move(sock)),
-            std::move(evb),
+        QuicServerTransport(
-            std::move(sock),
+            evb,
+            nullptr /* Initialized through the QuicTransportBaseLite constructor
+                     */
+            ,
             nullptr,
             nullptr,
             nullptr) {}

quic/server/test/QuicServerTransportTestUtil.h

@@ -36,9 +36,12 @@ class TestingQuicServerTransport : public QuicServerTransport {
       ConnectionSetupCallback* connSetupCb,
       ConnectionCallback* connCb,
       std::shared_ptr<const fizz::server::FizzServerContext> ctx)
-      : QuicServerTransport(
+      : QuicTransportBaseLite(evb, std::move(sock)),
-            std::move(evb),
+        QuicServerTransport(
-            std::move(sock),
+            evb,
+            nullptr /* Initialized through the QuicTransportBaseLite constructor
+                     */
+            ,
             connSetupCb,
             connCb,
             std::move(ctx)) {}