test: remove the manual declarations of TestThread and figure it out from the platform.

source/common/common/thread.cc

@@ -20,15 +20,20 @@ namespace {
 // call-sites for isMainThread(), which might be a bit of work, but will make
 // tests more hermetic.
 struct ThreadIds {
-  // Determines whether we are currently running on the main-thread or
-  // test-thread. We need to allow for either one because we don't establish
-  // the full threading model in all unit tests.
-  bool inMainOrTestThread() const {
-    // We don't take the lock when testing the thread IDs, as they are atomic,
-    // and are cleared when being released. All possible thread orderings
-    // result in the correct result even without a lock.
-    std::thread::id id = std::this_thread::get_id();
-    return main_thread_id_ == id || test_thread_id_ == id;
+  // We don't take the lock when testing the thread IDs, as they are atomic,
+  // and are cleared when being released. All possible thread orderings
+  // result in the correct result even without a lock.
+  bool inMainThread() const { return std::this_thread::get_id() == main_thread_id_; }
+
+  bool inTestThread() const {
+    // https://stackoverflow.com/questions/4867839/how-can-i-tell-if-pthread-self-is-the-main-first-thread-in-the-process
+#ifdef __linux__
+    return getpid() == syscall(SYS_gettid);
+#elif defined(__APPLE__)
+    return pthread_main_np() != 0;
+#else
+    return true;
+#endif
   }
 
   bool isMainThreadActive() const {
@@ -53,20 +58,6 @@ struct ThreadIds {
     }
   }
 
-  // Call this when the TestThread exits. Nested semantics are supported, so
-  // that if multiple TestThread instances are declared, we unwind them
-  // properly.
-  void releaseTestThread() {
-    absl::MutexLock lock(&mutex_);
-    ASSERT(test_thread_use_count_ > 0);
-    ASSERT(std::this_thread::get_id() == test_thread_id_);
-    if (--test_thread_use_count_ == 0) {
-      // Clearing the thread ID when its use-count goes to zero allows us
-      // to read the atomic without taking a lock.
-      test_thread_id_ = std::thread::id{};
-    }
-  }
-
   // Declares current thread as the main one, or verifies that the current
   // thread matches any previous declarations.
   void registerMainThread() {
@@ -78,39 +69,24 @@ struct ThreadIds {
     }
   }
 
-  // Declares current thread as the test thread, or verifies that the current
-  // thread matches any previous declarations.
-  void registerTestThread() {
-    absl::MutexLock lock(&mutex_);
-    if (++test_thread_use_count_ > 1) {
-      ASSERT(std::this_thread::get_id() == test_thread_id_);
-    } else {
-      test_thread_id_ = std::this_thread::get_id();
-    }
-  }
-
 private:
   // The atomic thread IDs can be read without a mutex, but they are written
   // under a mutex so that they are consistent with their use_counts. this
   // avoids the possibility of two threads racing to claim being the main/test
   // thread.
   std::atomic<std::thread::id> main_thread_id_;
-  std::atomic<std::thread::id> test_thread_id_;
 
   int32_t main_thread_use_count_ GUARDED_BY(mutex_) = 0;
-  int32_t test_thread_use_count_ GUARDED_BY(mutex_) = 0;
   mutable absl::Mutex mutex_;
 };
 
 } // namespace
 
-bool MainThread::isMainOrTestThread() { return ThreadIds::get().inMainOrTestThread(); }
-
-bool MainThread::isMainThreadActive() { return ThreadIds::get().isMainThreadActive(); }
+bool MainThread::isMainThread() { return ThreadIds::get().inMainThread(); }
 
-TestThread::TestThread() { ThreadIds::get().registerTestThread(); }
+bool MainThread::isTestThread() { return ThreadIds::get().inTestThread(); }
 
-TestThread::~TestThread() { ThreadIds::get().releaseTestThread(); }
+bool MainThread::isMainThreadActive() { return ThreadIds::get().isMainThreadActive(); }
 
 MainThread::MainThread() { ThreadIds::get().registerMainThread(); }
 

source/common/common/thread.h

@@ -168,17 +168,12 @@ class AtomicPtr : private AtomicPtrArray<T, 1, alloc_mode> {
   T* get(const MakeObject& make_object) { return BaseClass::get(0, make_object); }
 };
 
-// RAII object to declare the TestThread. This should be declared in main() or
-// equivalent for any test binaries.
+// Deprecated RAII object to declare the TestThread. This is no longer needed as
+// we figure this out automatically from the platform.
 //
-// Generally we expect TestThread to be instantiated only once on main() for
-// each test binary, though nested instantiations are allowed as long as the
-// thread ID does not change.
-class TestThread {
-public:
-  TestThread();
-  ~TestThread();
-};
+// TODO(jmarantz): Remove this declaration after Nov 15, 2021 to allow
+// references from other repositories to be cleaned up.
+struct TestThread {};
 
 // RAII object to declare the MainThread. This should be declared in the thread
 // function or equivalent.
@@ -198,8 +193,25 @@ class MainThread {
 
   /**
    * @return whether the current thread is the main thread or test thread.
+   *
+   * Determines whether we are currently running on the main-thread or
+   * test-thread. We need to allow for either one because we don't establish
+   * the full threading model in all unit tests.
+   */
+  static bool isMainOrTestThread() { return isMainThread() || isTestThread(); }
+
+  /**
+   * @return whether the current thread is the main thread.
+   */
+  static bool isMainThread();
+
+  /**
+   * @return whether the current thread is the test thread.
+   *
+   * Note, on the Windows platform, this always returns true. On MacOS and
+   * Linux, system-dependent calls are used to detect the test-thread.
    */
-  static bool isMainOrTestThread();
+  static bool isTestThread();
 
   /**
    * @return whether a MainThread has been instantiated.

test/benchmark/main.cc

@@ -43,7 +43,6 @@ int main(int argc, char** argv) {
   }
 
   TestEnvironment::initializeTestMain(argv[0]);
-  Thread::TestThread test_thread;
 
   // Suppressing non-error messages in benchmark tests. This hides warning
   // messages that appear when using a runtime feature when there isn't an initialized

test/common/thread_local/thread_local_impl_test.cc

@@ -16,17 +16,22 @@ namespace Envoy {
 namespace ThreadLocal {
 
 TEST(MainThreadVerificationTest, All) {
-  // Before threading is on, assertion on main thread should be true.
+  // Before threading is on, we are in the test thread, not the main thread.
+  EXPECT_FALSE(Thread::MainThread::isMainThread());
+  EXPECT_TRUE(Thread::MainThread::isTestThread());
   EXPECT_TRUE(Thread::MainThread::isMainOrTestThread());
   {
     InstanceImpl tls;
     // Tls instance has been initialized.
     // Call to main thread verification should succeed in main thread.
+    EXPECT_TRUE(Thread::MainThread::isMainThread());
     EXPECT_TRUE(Thread::MainThread::isMainOrTestThread());
     tls.shutdownGlobalThreading();
     tls.shutdownThread();
   }
-  // After threading is off, assertion on main thread should be true.
+  // After threading is off, assertion we are again in the test thread, not the main thread.
+  EXPECT_FALSE(Thread::MainThread::isMainThread());
+  EXPECT_TRUE(Thread::MainThread::isTestThread());
   EXPECT_TRUE(Thread::MainThread::isMainOrTestThread());
 }
 
@@ -301,7 +306,7 @@ TEST(ThreadLocalInstanceImplDispatcherTest, Dispatcher) {
         // Verify we have the expected dispatcher for the new thread thread.
         EXPECT_EQ(thread_dispatcher.get(), &tls.dispatcher());
         // Verify that it is inside the worker thread.
-        EXPECT_FALSE(Thread::MainThread::isMainOrTestThread());
+        EXPECT_FALSE(Thread::MainThread::isMainThread());
       });
   thread->join();
 

test/fuzz/fuzz_runner.cc

@@ -31,8 +31,6 @@ void Runner::setupEnvironment(int argc, char** argv, spdlog::level::level_enum d
   // state.
   ProcessWide process_wide;
   TestEnvironment::initializeOptions(argc, argv);
-  static auto* test_thread = new Envoy::Thread::TestThread;
-  UNREFERENCED_PARAMETER(test_thread);
 
   const auto environment_log_level = TestEnvironment::getOptions().logLevel();
   // We only override the default log level if it looks like we're debugging;

test/integration/base_integration_test.h

@@ -364,8 +364,6 @@ class BaseIntegrationTest : protected Logger::Loggable<Logger::Id::testing> {
 
   std::unique_ptr<Stats::Scope> upstream_stats_store_;
 
-  Thread::TestThread test_thread_;
-
   // Make sure the test server will be torn down after any fake client.
   // The test server owns the runtime, which is often accessed by client and
   // fake upstream codecs and must outlast them.

test/test_runner.cc

@@ -73,8 +73,6 @@ class RuntimeManagingListener : public ::testing::EmptyTestEventListener {
 } // namespace
 
 int TestRunner::RunTests(int argc, char** argv) {
-  Thread::TestThread test_thread;
-
   ::testing::InitGoogleMock(&argc, argv);
   // We hold on to process_wide to provide RAII cleanup of process-wide
   // state.

test/tools/router_check/router_check.cc

@@ -9,7 +9,6 @@
 #include "test/tools/router_check/router.h"
 
 int main(int argc, char* argv[]) {
-  Envoy::Thread::TestThread test_thread;
   Envoy::Options options(argc, argv);
 
   const bool enforce_coverage = options.failUnder() != 0.0;