From 577c580cd03bd4b4af16827a66929451e5e7b466 Mon Sep 17 00:00:00 2001 From: Danil Chapovalov Date: Thu, 31 Oct 2019 12:33:17 +0100 Subject: [PATCH] Do not stop SingleThreadedTaskQueueForTestingTest near the end of the tests That brings usage of that queue closer to the production. In particular that should surface race conditions on destruction. Those should be fixed rather than avoided. Bug: webrtc:10933 Change-Id: Iff60cf5a4b87bd848117ef543ffc97f6504dc979 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/157898 Reviewed-by: Sebastian Jansson Commit-Queue: Danil Chapovalov Cr-Commit-Position: refs/heads/master@{#29665} --- call/rampup_tests.cc | 5 +---- test/call_test.cc | 23 +---------------------- 2 files changed, 2 insertions(+), 26 deletions(-) diff --git a/call/rampup_tests.cc b/call/rampup_tests.cc index 9c7a2678af..0377126821 100644 --- a/call/rampup_tests.cc +++ b/call/rampup_tests.cc @@ -91,10 +91,7 @@ RampUpTester::RampUpTester(size_t num_video_streams, EXPECT_LE(num_audio_streams_, 1u); } -RampUpTester::~RampUpTester() { - // Special case for WebRTC-QuickPerfTest/Enabled/ - SendTask(RTC_FROM_HERE, task_queue_, [this] { pending_task_.Stop(); }); -} +RampUpTester::~RampUpTester() = default; void RampUpTester::ModifySenderBitrateConfig( BitrateConstraints* bitrate_config) { diff --git a/test/call_test.cc b/test/call_test.cc index 51ddaa32f8..d83f87a8c6 100644 --- a/test/call_test.cc +++ b/test/call_test.cc @@ -58,19 +58,7 @@ CallTest::CallTest() audio_encoder_factory_(CreateBuiltinAudioEncoderFactory()), task_queue_("CallTestTaskQueue") {} -CallTest::~CallTest() { - // In most cases the task_queue_ should have been stopped by now, assuming - // the regular path of using CallTest to call PerformTest (followed by - // cleanup). However, there are some tests that don't use the class that way - // hence we need this special handling for cleaning up. - if (task_queue_.IsRunning()) { - SendTask(RTC_FROM_HERE, &task_queue_, [this]() { - fake_send_audio_device_ = nullptr; - fake_recv_audio_device_ = nullptr; - video_sources_.clear(); - }); - } -} +CallTest::~CallTest() = default; void CallTest::RegisterRtpExtension(const RtpExtension& extension) { for (const RtpExtension& registered_extension : rtp_extensions_) { @@ -209,15 +197,6 @@ void CallTest::RunBaseTest(BaseTest* test) { fake_send_audio_device_ = nullptr; fake_recv_audio_device_ = nullptr; }); - - // To avoid a race condition during destruction, which can happen while - // a derived class is being destructed but pending tasks might still run - // because the |task_queue_| is still in scope, we stop the TQ here. - // Note that tests should not be posting more tasks during teardown but - // as is, that's hard to control with the current test harness. E.g. transport - // classes continue to issue callbacks (e.g. OnSendRtp) during teardown, which - // can have a ripple effect. - task_queue_.Stop(); } void CallTest::CreateCalls() {