diff --git a/bazel/external/quiche.BUILD b/bazel/external/quiche.BUILD index c28f72848e139..aa9461abf1a3b 100644 --- a/bazel/external/quiche.BUILD +++ b/bazel/external/quiche.BUILD @@ -192,6 +192,14 @@ cc_library( deps = ["@envoy//test/extensions/quic_listeners/quiche/platform:quic_platform_test_output_impl_lib"], ) +cc_library( + name = "quic_platform_thread", + testonly = 1, + hdrs = ["quiche/quic/platform/api/quic_thread.h"], + visibility = ["//visibility:public"], + deps = ["@envoy//test/extensions/quic_listeners/quiche/platform:quic_platform_thread_impl_lib"], +) + cc_library( name = "quic_platform_base", hdrs = [ @@ -235,7 +243,6 @@ cc_library( "quiche/quic/platform/api/quic_stack_trace.h", "quiche/quic/platform/api/quic_string_utils.h", "quiche/quic/platform/api/quic_text_utils.h", - "quiche/quic/platform/api/quic_thread.h", ], "@envoy", ), diff --git a/bazel/external/quiche.genrule_cmd b/bazel/external/quiche.genrule_cmd index a1946703a2929..5d3528cca6bc1 100644 --- a/bazel/external/quiche.genrule_cmd +++ b/bazel/external/quiche.genrule_cmd @@ -24,6 +24,7 @@ cat <sed_commands /^#include/ s!net/quic/platform/impl/quic_port_utils_impl.h!test/extensions/quic_listeners/quiche/platform/quic_port_utils_impl.h! /^#include/ s!net/quic/platform/impl/quic_test_impl.h!test/extensions/quic_listeners/quiche/platform/quic_test_impl.h! /^#include/ s!net/quic/platform/impl/quic_test_output_impl.h!test/extensions/quic_listeners/quiche/platform/quic_test_output_impl.h! +/^#include/ s!net/quic/platform/impl/quic_thread_impl.h!test/extensions/quic_listeners/quiche/platform/quic_thread_impl.h! # Rewrite include directives for platform impl files. /^#include/ s!net/(http2|spdy|quic)/platform/impl/!extensions/quic_listeners/quiche/platform/! diff --git a/source/extensions/quic_listeners/quiche/platform/BUILD b/source/extensions/quic_listeners/quiche/platform/BUILD index c3026500517ae..2f1ebe8ef3475 100644 --- a/source/extensions/quic_listeners/quiche/platform/BUILD +++ b/source/extensions/quic_listeners/quiche/platform/BUILD @@ -100,7 +100,6 @@ envoy_cc_library( ] + envoy_select_quiche([ "quic_logging_impl.h", "quic_stack_trace_impl.h", - "quic_thread_impl.h", ]), external_deps = [ "abseil_base", @@ -117,7 +116,6 @@ envoy_cc_library( "@com_googlesource_quiche//:quic_buffer_allocator_lib", ] + envoy_select_quiche([ ":quic_platform_logging_impl_lib", - "//include/envoy/thread:thread_interface", "//source/common/common:assert_lib", "//source/common/common:byte_order_lib", "//source/server:backtrace_lib", diff --git a/test/extensions/quic_listeners/quiche/platform/BUILD b/test/extensions/quic_listeners/quiche/platform/BUILD index 68209c63a4b1a..7c83e36566626 100644 --- a/test/extensions/quic_listeners/quiche/platform/BUILD +++ b/test/extensions/quic_listeners/quiche/platform/BUILD @@ -3,7 +3,6 @@ licenses(["notice"]) # Apache 2 load( "//bazel:envoy_build_system.bzl", "envoy_cc_fuzz_test", - "envoy_cc_platform_dep", "envoy_cc_test", "envoy_cc_test_binary", "envoy_cc_test_library", @@ -44,7 +43,8 @@ envoy_cc_test( "@com_googlesource_quiche//:quic_platform_port_utils", "@com_googlesource_quiche//:quic_platform_sleep", "@com_googlesource_quiche//:quic_platform_test_output", - ] + envoy_cc_platform_dep("//source/exe:platform_impl_lib"), + "@com_googlesource_quiche//:quic_platform_thread", + ], ) envoy_cc_test( @@ -82,6 +82,16 @@ envoy_cc_test_library( ], ) +envoy_cc_test_library( + name = "quic_platform_thread_impl_lib", + hdrs = ["quic_thread_impl.h"], + deps = [ + "//include/envoy/thread:thread_interface", + "//source/common/common:assert_lib", + "//test/test_common:thread_factory_for_test_lib", + ], +) + envoy_cc_test_library( name = "quic_platform_test_impl_lib", hdrs = ["quic_test_impl.h"], diff --git a/test/extensions/quic_listeners/quiche/platform/quic_platform_test.cc b/test/extensions/quic_listeners/quiche/platform/quic_platform_test.cc index bf341723b94a0..df87edca85dd4 100644 --- a/test/extensions/quic_listeners/quiche/platform/quic_platform_test.cc +++ b/test/extensions/quic_listeners/quiche/platform/quic_platform_test.cc @@ -12,8 +12,6 @@ #include "common/memory/stats.h" #include "common/network/utility.h" -#include "exe/platform_impl.h" - #include "test/common/stats/stat_test_utility.h" #include "test/extensions/transport_sockets/tls/ssl_test_utility.h" #include "test/mocks/api/mocks.h" @@ -254,14 +252,10 @@ TEST_F(QuicPlatformTest, QuicStringPiece) { } TEST_F(QuicPlatformTest, QuicThread) { - Envoy::PlatformImpl platform_impl; - class AdderThread : public QuicThread { public: - AdderThread(int* value, int increment, Envoy::Thread::ThreadFactory& thread_factory) - : QuicThread("adder_thread"), value_(value), increment_(increment) { - setThreadFactory(thread_factory); - } + AdderThread(int* value, int increment) + : QuicThread("adder_thread"), value_(value), increment_(increment) {} ~AdderThread() override = default; @@ -276,19 +270,19 @@ TEST_F(QuicPlatformTest, QuicThread) { int value = 0; // A QuicThread that is never started, which is ok. - { AdderThread t0(&value, 1, platform_impl.threadFactory()); } + { AdderThread t0(&value, 1); } EXPECT_EQ(0, value); // A QuicThread that is started and joined as usual. { - AdderThread t1(&value, 1, platform_impl.threadFactory()); + AdderThread t1(&value, 1); t1.Start(); t1.Join(); } EXPECT_EQ(1, value); // QuicThread will panic if it's started but not joined. - EXPECT_DEATH_LOG_TO_STDERR({ AdderThread(&value, 2, platform_impl.threadFactory()).Start(); }, + EXPECT_DEATH_LOG_TO_STDERR({ AdderThread(&value, 2).Start(); }, "QuicThread should be joined before destruction"); } diff --git a/source/extensions/quic_listeners/quiche/platform/quic_thread_impl.h b/test/extensions/quic_listeners/quiche/platform/quic_thread_impl.h similarity index 76% rename from source/extensions/quic_listeners/quiche/platform/quic_thread_impl.h rename to test/extensions/quic_listeners/quiche/platform/quic_thread_impl.h index bf2b63419a5b4..e2d87917b1c33 100644 --- a/source/extensions/quic_listeners/quiche/platform/quic_thread_impl.h +++ b/test/extensions/quic_listeners/quiche/platform/quic_thread_impl.h @@ -12,6 +12,8 @@ #include "common/common/assert.h" +#include "test/test_common/thread_factory_for_test.h" + #include "absl/synchronization/notification.h" namespace quic { @@ -19,7 +21,9 @@ namespace quic { // A class representing a thread of execution in QUIC. class QuicThreadImpl { public: - QuicThreadImpl(const std::string& /*name*/) {} + QuicThreadImpl(const std::string& /*name*/) + : thread_factory_(Envoy::Thread::threadFactoryForTest()) {} + QuicThreadImpl(const QuicThreadImpl&) = delete; QuicThreadImpl& operator=(const QuicThreadImpl&) = delete; @@ -30,11 +34,10 @@ class QuicThreadImpl { } void Start() { - ASSERT(thread_factory_ != nullptr); if (thread_ != nullptr || thread_is_set_.HasBeenNotified()) { PANIC("QuicThread can only be started once."); } - thread_ = thread_factory_->createThread([this]() { + thread_ = thread_factory_.createThread([this]() { thread_is_set_.WaitForNotification(); this->Run(); }); @@ -49,14 +52,6 @@ class QuicThreadImpl { thread_ = nullptr; } - // Sets the thread factory to use. - // NOTE: The factory can not be passed via a constructor argument because this class is itself a - // dependency of an external library that derives from it and expects a single argument - // constructor. - void setThreadFactory(Envoy::Thread::ThreadFactory& thread_factory) { - thread_factory_ = &thread_factory; - } - protected: virtual void Run() { // We don't want this function to be pure virtual, because it will be called if: @@ -70,7 +65,7 @@ class QuicThreadImpl { private: Envoy::Thread::ThreadPtr thread_; - Envoy::Thread::ThreadFactory* thread_factory_; + Envoy::Thread::ThreadFactory& thread_factory_; absl::Notification thread_is_set_; // Whether |thread_| is set in parent. };