Set up a new rtc::Thread instance per test.
Several tests leave pending tasks behind after executing, which may
affect the state of subsequent tests. This CL isolates each test in
the sense that a dedicated Thread instance is created per test and
then pending tasks are flushed and the Thread instance deleted.
Down the line we may want to improve on this and flag those tests
that leave pending tasks/timers etc.
Change-Id: Ibaf3719a9974c57ac2169edca0e2a06a9ea6c78f
Bug: webrtc:11574
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/175132
Commit-Queue: Tommi <tommi@webrtc.org>
Reviewed-by: Artem Titov <titovartem@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31268}
diff --git a/api/BUILD.gn b/api/BUILD.gn
index ebd22c6..70caec6 100644
--- a/api/BUILD.gn
+++ b/api/BUILD.gn
@@ -407,7 +407,7 @@
deps = [
":video_quality_test_fixture_api",
"../rtc_base:checks",
- "../rtc_base:thread_checker",
+ "../rtc_base:platform_thread_types",
]
}
diff --git a/api/test/test_dependency_factory.cc b/api/test/test_dependency_factory.cc
index e72f55a..41ad70c 100644
--- a/api/test/test_dependency_factory.cc
+++ b/api/test/test_dependency_factory.cc
@@ -14,22 +14,24 @@
#include <utility>
#include "rtc_base/checks.h"
-#include "rtc_base/thread_checker.h"
+#include "rtc_base/platform_thread_types.h"
namespace webrtc {
+namespace {
// This checks everything in this file gets called on the same thread. It's
// static because it needs to look at the static methods too.
-rtc::ThreadChecker* GetThreadChecker() {
- static rtc::ThreadChecker checker;
- return &checker;
+bool IsValidTestDependencyFactoryThread() {
+ const rtc::PlatformThreadRef main_thread = rtc::CurrentThreadRef();
+ return rtc::IsThreadRefEqual(main_thread, rtc::CurrentThreadRef());
}
+} // namespace
std::unique_ptr<TestDependencyFactory> TestDependencyFactory::instance_ =
nullptr;
const TestDependencyFactory& TestDependencyFactory::GetInstance() {
- RTC_DCHECK(GetThreadChecker()->IsCurrent());
+ RTC_DCHECK(IsValidTestDependencyFactoryThread());
if (instance_ == nullptr) {
instance_ = std::make_unique<TestDependencyFactory>();
}
@@ -38,14 +40,14 @@
void TestDependencyFactory::SetInstance(
std::unique_ptr<TestDependencyFactory> instance) {
- RTC_DCHECK(GetThreadChecker()->IsCurrent());
+ RTC_DCHECK(IsValidTestDependencyFactoryThread());
RTC_CHECK(instance_ == nullptr);
instance_ = std::move(instance);
}
std::unique_ptr<VideoQualityTestFixtureInterface::InjectionComponents>
TestDependencyFactory::CreateComponents() const {
- RTC_DCHECK(GetThreadChecker()->IsCurrent());
+ RTC_DCHECK(IsValidTestDependencyFactoryThread());
return nullptr;
}
diff --git a/call/rampup_tests.cc b/call/rampup_tests.cc
index 64eab05..89fbe3d 100644
--- a/call/rampup_tests.cc
+++ b/call/rampup_tests.cc
@@ -362,14 +362,14 @@
void RampUpTester::TriggerTestDone() {
RTC_DCHECK_GE(test_start_ms_, 0);
- // TODO(holmer): Add audio send stats here too when those APIs are available.
- if (!send_stream_)
- return;
-
// Stop polling stats.
// Corner case for field_trials=WebRTC-QuickPerfTest/Enabled/
SendTask(RTC_FROM_HERE, task_queue_, [this] { pending_task_.Stop(); });
+ // TODO(holmer): Add audio send stats here too when those APIs are available.
+ if (!send_stream_)
+ return;
+
VideoSendStream::Stats send_stats = send_stream_->GetStats();
send_stream_ = nullptr; // To avoid dereferencing a bad pointer.
diff --git a/test/test_main_lib.cc b/test/test_main_lib.cc
index 15318b4..1fd6230 100644
--- a/test/test_main_lib.cc
+++ b/test/test_main_lib.cc
@@ -100,6 +100,40 @@
class TestMainImpl : public TestMain {
public:
+ // In order to set up a fresh rtc::Thread state for each test and avoid
+ // accidentally carrying over pending tasks that might be sent from one test
+ // and executed while another test is running, we inject a TestListener
+ // that sets up a new rtc::Thread instance for the main thread, per test.
+ class TestListener : public ::testing::EmptyTestEventListener {
+ public:
+ TestListener() = default;
+
+ private:
+ void OnTestStart(const ::testing::TestInfo& test_info) override {
+ // Ensure that main thread gets wrapped as an rtc::Thread.
+ // TODO(bugs.webrtc.org/9714): It might be better to avoid wrapping the
+ // main thread, or leave it to individual tests that need it. But as long
+ // as we have automatic thread wrapping, we need this to avoid that some
+ // other random thread (which one depending on which tests are run) gets
+ // automatically wrapped.
+ thread_ = rtc::Thread::CreateWithSocketServer();
+ thread_->WrapCurrent();
+ RTC_DCHECK_EQ(rtc::Thread::Current(), thread_.get());
+ }
+
+ void OnTestEnd(const ::testing::TestInfo& test_info) override {
+ // Terminate the message loop. Note that if the test failed to clean
+ // up pending messages, this may execute part of the test. Ideally we
+ // should print a warning message here, or even fail the test if it leaks.
+ thread_->Quit(); // Signal quit.
+ thread_->Run(); // Flush + process Quit signal.
+ thread_->UnwrapCurrent();
+ thread_ = nullptr;
+ }
+
+ std::unique_ptr<rtc::Thread> thread_;
+ };
+
int Init(int* argc, char* argv[]) override {
::testing::InitGoogleMock(argc, argv);
absl::ParseCommandLine(*argc, argv);
@@ -134,14 +168,7 @@
rtc::InitializeSSL();
rtc::SSLStreamAdapter::EnableTimeCallbackForTesting();
- // Ensure that main thread gets wrapped as an rtc::Thread.
- // TODO(bugs.webrt.org/9714): It might be better to avoid wrapping the main
- // thread, or leave it to individual tests that need it. But as long as we
- // have automatic thread wrapping, we need this to avoid that some other
- // random thread (which one depending on which tests are run) gets
- // automatically wrapped.
- rtc::ThreadManager::Instance()->WrapCurrentThread();
- RTC_CHECK(rtc::Thread::Current());
+ ::testing::UnitTest::GetInstance()->listeners().Append(new TestListener());
return 0;
}
