From ea9ae5b8bc6ddcc67a88bd3692436b1ccfeac92a Mon Sep 17 00:00:00 2001 From: Mirko Bonadei Date: Thu, 8 Jul 2021 13:07:51 +0200 Subject: [PATCH] Destroy threads and TaskQueue at the end of tests. On ASan, SimulatedRealTimeControllerConformanceTest is flaky and triggers `stack-use-after-scope` because on some occasions, the delayed callback gets invoked when the test is tearing down (the callback holds a reference to an object allocated on the test function stack). This CL ensures threads and TaskQueues are stopped when the tests scope is exited. Some flakiness might remain on realtime tests but that can only be addressed by increasing the wait. Bug: webrtc:12954 Change-Id: I4ac1a6682e18bb144a3aeb03921a805e3fb39b2d Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/225422 Reviewed-by: Andrey Logvin Reviewed-by: Danil Chapovalov Commit-Queue: Mirko Bonadei Cr-Commit-Position: refs/heads/master@{#34437} --- .../time_controller_conformance_test.cc | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/test/time_controller/time_controller_conformance_test.cc b/test/time_controller/time_controller_conformance_test.cc index 10f0e1d724..3d582cad8e 100644 --- a/test/time_controller/time_controller_conformance_test.cc +++ b/test/time_controller/time_controller_conformance_test.cc @@ -92,6 +92,9 @@ TEST_P(SimulatedRealTimeControllerConformanceTest, ThreadPostOrderTest) { thread->PostTask(RTC_FROM_HERE, [&]() { execution_order.Executed(2); }); time_controller->AdvanceTime(TimeDelta::Millis(100)); EXPECT_THAT(execution_order.order(), ElementsAreArray({1, 2})); + // Destroy `thread` before `execution_order` to be sure `execution_order` + // is not accessed on the posted task after it is destroyed. + thread = nullptr; } TEST_P(SimulatedRealTimeControllerConformanceTest, ThreadPostDelayedOrderTest) { @@ -105,6 +108,9 @@ TEST_P(SimulatedRealTimeControllerConformanceTest, ThreadPostDelayedOrderTest) { thread->PostTask(ToQueuedTask([&]() { execution_order.Executed(1); })); time_controller->AdvanceTime(TimeDelta::Millis(600)); EXPECT_THAT(execution_order.order(), ElementsAreArray({1, 2})); + // Destroy `thread` before `execution_order` to be sure `execution_order` + // is not accessed on the posted task after it is destroyed. + thread = nullptr; } TEST_P(SimulatedRealTimeControllerConformanceTest, ThreadPostInvokeOrderTest) { @@ -119,6 +125,9 @@ TEST_P(SimulatedRealTimeControllerConformanceTest, ThreadPostInvokeOrderTest) { thread->Invoke(RTC_FROM_HERE, [&]() { execution_order.Executed(2); }); time_controller->AdvanceTime(TimeDelta::Millis(100)); EXPECT_THAT(execution_order.order(), ElementsAreArray({1, 2})); + // Destroy `thread` before `execution_order` to be sure `execution_order` + // is not accessed on the posted task after it is destroyed. + thread = nullptr; } TEST_P(SimulatedRealTimeControllerConformanceTest, @@ -136,6 +145,9 @@ TEST_P(SimulatedRealTimeControllerConformanceTest, }); time_controller->AdvanceTime(TimeDelta::Millis(100)); EXPECT_THAT(execution_order.order(), ElementsAreArray({1, 2})); + // Destroy `thread` before `execution_order` to be sure `execution_order` + // is not accessed on the posted task after it is destroyed. + thread = nullptr; } TEST_P(SimulatedRealTimeControllerConformanceTest, @@ -158,6 +170,9 @@ TEST_P(SimulatedRealTimeControllerConformanceTest, /*warn_after_ms=*/10'000)); time_controller->AdvanceTime(TimeDelta::Millis(100)); EXPECT_THAT(execution_order.order(), ElementsAreArray({1, 2})); + // Destroy `task_queue` before `execution_order` to be sure `execution_order` + // is not accessed on the posted task after it is destroyed. + task_queue = nullptr; } INSTANTIATE_TEST_SUITE_P(ConformanceTest,