quic: fix test flake due to no socket interface

Fixes envoyproxy#35467 Signed-off-by: Greg Greenway <ggreenway@apple.com>
ggreenway · Jul 31, 2024 · 86fabb4 · 86fabb4
1 parent 7b8132a
commit 86fabb4
Show file tree

Hide file tree

Showing 5 changed files with 20 additions and 29 deletions.
diff --git a/source/common/singleton/threadsafe_singleton.h b/source/common/singleton/threadsafe_singleton.h
@@ -66,6 +66,9 @@ template <class T> class InjectableSingleton {
   }
   static void clear() { loader_ = nullptr; }
 
+  // Atomically replace the value, returning the old value.
+  static T* replaceForTest(T* new_value) { return loader_.exchange(new_value); }
+
 protected:
   static std::atomic<T*> loader_;
 };
@@ -86,20 +89,16 @@ template <class T> class ScopedInjectableLoader {
   std::unique_ptr<T> instance_;
 };
 
-// This class saves the singleton object and restore the original singleton at destroy. This class
-// is not thread safe. It can be used in single thread test.
-template <class T>
-class StackedScopedInjectableLoader :
-    // To access the protected loader_.
-    protected InjectableSingleton<T> {
+// This class saves the singleton object and restore the original singleton at destroy.
+template <class T> class StackedScopedInjectableLoaderForTest {
 public:
-  explicit StackedScopedInjectableLoader(std::unique_ptr<T>&& instance) {
-    original_loader_ = InjectableSingleton<T>::getExisting();
-    InjectableSingleton<T>::clear();
+  explicit StackedScopedInjectableLoaderForTest(std::unique_ptr<T>&& instance) {
     instance_ = std::move(instance);
-    InjectableSingleton<T>::initialize(instance_.get());
+    original_loader_ = InjectableSingleton<T>::replaceForTest(instance_.get());
+  }
+  ~StackedScopedInjectableLoaderForTest() {
+    InjectableSingleton<T>::replaceForTest(original_loader_);
   }
-  ~StackedScopedInjectableLoader() { InjectableSingleton<T>::loader_ = original_loader_; }
 
 private:
   std::unique_ptr<T> instance_;

diff --git a/test/common/listener_manager/listener_manager_impl_test.cc b/test/common/listener_manager/listener_manager_impl_test.cc
@@ -2604,7 +2604,8 @@ TEST_P(ListenerManagerImplTest, BindToPortEqualToFalse) {
   InSequence s;
   auto mock_interface = std::make_unique<Network::MockSocketInterface>(
       std::vector<Network::Address::IpVersion>{Network::Address::IpVersion::v4});
-  StackedScopedInjectableLoader<Network::SocketInterface> new_interface(std::move(mock_interface));
+  StackedScopedInjectableLoaderForTest<Network::SocketInterface> new_interface(
+      std::move(mock_interface));
 
   ProdListenerComponentFactory real_listener_factory(server_);
   EXPECT_CALL(*worker_, start(_, _));
@@ -2643,7 +2644,8 @@ TEST_P(ListenerManagerImplTest, UpdateBindToPortEqualToFalse) {
   InSequence s;
   auto mock_interface = std::make_unique<Network::MockSocketInterface>(
       std::vector<Network::Address::IpVersion>{Network::Address::IpVersion::v4});
-  StackedScopedInjectableLoader<Network::SocketInterface> new_interface(std::move(mock_interface));
+  StackedScopedInjectableLoaderForTest<Network::SocketInterface> new_interface(
+      std::move(mock_interface));
 
   ProdListenerComponentFactory real_listener_factory(server_);
   EXPECT_CALL(*worker_, start(_, _));

diff --git a/test/common/network/listen_socket_impl_test.cc b/test/common/network/listen_socket_impl_test.cc
@@ -229,7 +229,7 @@ TEST_P(ListenSocketImplTestTcp, SupportedIpFamilyVirtualSocketIsCreatedWithNoBsd
   auto any_address = version_ == Address::IpVersion::v4 ? Utility::getIpv4AnyAddress()
                                                         : Utility::getIpv6AnyAddress();
 
-  StackedScopedInjectableLoader<SocketInterface> new_interface(std::move(mock_interface));
+  StackedScopedInjectableLoaderForTest<SocketInterface> new_interface(std::move(mock_interface));
 
   {
     EXPECT_CALL(*mock_interface_ptr, socket(_, _, _)).Times(0);
@@ -245,7 +245,7 @@ TEST_P(ListenSocketImplTestTcp, DeathAtUnSupportedIpFamilyListenSocket) {
   auto* mock_interface_ptr = mock_interface.get();
   auto the_other_address = version_ == Address::IpVersion::v4 ? Utility::getIpv6AnyAddress()
                                                               : Utility::getIpv4AnyAddress();
-  StackedScopedInjectableLoader<SocketInterface> new_interface(std::move(mock_interface));
+  StackedScopedInjectableLoaderForTest<SocketInterface> new_interface(std::move(mock_interface));
   {
     EXPECT_CALL(*mock_interface_ptr, socket(_, _, _)).Times(0);
     EXPECT_CALL(*mock_interface_ptr, socket(_, _, _, _, _)).Times(0);

diff --git a/test/integration/socket_interface_swap.cc b/test/integration/socket_interface_swap.cc
@@ -3,10 +3,8 @@
 namespace Envoy {
 
 SocketInterfaceSwap::SocketInterfaceSwap(Network::Socket::Type socket_type)
-    : write_matcher_(std::make_shared<IoHandleMatcher>(socket_type)) {
-  Envoy::Network::SocketInterfaceSingleton::clear();
-  test_socket_interface_loader_ = std::make_unique<Envoy::Network::SocketInterfaceLoader>(
-      std::make_unique<Envoy::Network::TestSocketInterface>(
+    : write_matcher_(std::make_shared<IoHandleMatcher>(socket_type)),
+      test_socket_interface_loader_(std::make_unique<Envoy::Network::TestSocketInterface>(
           [write_matcher = write_matcher_](Envoy::Network::TestIoSocketHandle* io_handle)
               -> absl::optional<Api::IoCallUint64Result> {
             Api::IoErrorPtr error_override = write_matcher->returnConnectOverride(io_handle);
@@ -28,8 +26,7 @@ SocketInterfaceSwap::SocketInterfaceSwap(Network::Socket::Type socket_type)
           },
           [write_matcher = write_matcher_](Network::IoHandle::RecvMsgOutput& output) {
             write_matcher->readOverride(output);
-          }));
-}
+          })) {}
 
 void SocketInterfaceSwap::IoHandleMatcher::setResumeWrites() {
   absl::MutexLock lock(&mutex_);

diff --git a/test/integration/socket_interface_swap.h b/test/integration/socket_interface_swap.h
@@ -116,15 +116,8 @@ class SocketInterfaceSwap {
 
   explicit SocketInterfaceSwap(Network::Socket::Type socket_type);
 
-  ~SocketInterfaceSwap() {
-    test_socket_interface_loader_.reset();
-    Envoy::Network::SocketInterfaceSingleton::initialize(previous_socket_interface_);
-  }
-
-  Envoy::Network::SocketInterface* const previous_socket_interface_{
-      Envoy::Network::SocketInterfaceSingleton::getExisting()};
   std::shared_ptr<IoHandleMatcher> write_matcher_;
-  std::unique_ptr<Envoy::Network::SocketInterfaceLoader> test_socket_interface_loader_;
+  StackedScopedInjectableLoaderForTest<Network::SocketInterface> test_socket_interface_loader_;
 };
 
 } // namespace Envoy