From a497d12a02b487881b88fb8a5ad6823af9d21d1a Mon Sep 17 00:00:00 2001 From: Sebastian Jansson Date: Mon, 4 Feb 2019 16:39:28 +0100 Subject: [PATCH] Avoids PostTask to repost a repeated task. There seems to be a race caused by the libevent wrapping TaskQueue implementation when reposting a repeated task at destruction time. This race results in the posted task being leaked according to asan. Bug: webrtc:10278 Change-Id: Ida40b884547f3f789a804ca0ab3ce36982a4d68e Reviewed-on: https://webrtc-review.googlesource.com/c/121424 Reviewed-by: Karl Wiberg Reviewed-by: Ilya Nikolaevskiy Commit-Queue: Sebastian Jansson Cr-Commit-Position: refs/heads/master@{#26839} --- rtc_base/task_utils/repeating_task.cc | 8 +++----- rtc_base/task_utils/repeating_task_unittest.cc | 11 ++++++----- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/rtc_base/task_utils/repeating_task.cc b/rtc_base/task_utils/repeating_task.cc index c8cdf6a081..5f366cb000 100644 --- a/rtc_base/task_utils/repeating_task.cc +++ b/rtc_base/task_utils/repeating_task.cc @@ -38,12 +38,10 @@ bool RepeatingTaskBase::Run() { TimeDelta lost_time = Timestamp::us(rtc::TimeMicros()) - next_run_time_; next_run_time_ += delay; delay -= lost_time; + delay = std::max(delay, TimeDelta::Zero()); + + task_queue_->PostDelayedTask(absl::WrapUnique(this), delay.ms()); - if (delay <= TimeDelta::Zero()) { - task_queue_->PostTask(absl::WrapUnique(this)); - } else { - task_queue_->PostDelayedTask(absl::WrapUnique(this), delay.ms()); - } // Return false to tell the TaskQueue to not destruct this object since we // have taken ownership with absl::WrapUnique. return false; diff --git a/rtc_base/task_utils/repeating_task_unittest.cc b/rtc_base/task_utils/repeating_task_unittest.cc index 79bce0616a..52683e3d60 100644 --- a/rtc_base/task_utils/repeating_task_unittest.cc +++ b/rtc_base/task_utils/repeating_task_unittest.cc @@ -112,13 +112,14 @@ TEST(RepeatingTaskTest, CompensatesForShortRunTime) { rtc::TaskQueue task_queue("TestQueue"); RepeatingTaskHandle::Start(&task_queue, [&] { ++counter; - // Sleeping for the 5 ms should be compensated. - Sleep(TimeDelta::ms(5)); - return TimeDelta::ms(10); + // Sleeping for the 10 ms should be compensated. + Sleep(TimeDelta::ms(10)); + return TimeDelta::ms(30); }); - Sleep(TimeDelta::ms(15)); + Sleep(TimeDelta::ms(40)); + // We expect that the task have been called twice, once directly at Start and - // once after 10 ms has passed. + // once after 30 ms has passed. EXPECT_EQ(counter.load(), 2); }