From 5162dc337063e076da5736505f3446fa5e8efca0 Mon Sep 17 00:00:00 2001 From: Markus Handell Date: Thu, 5 Sep 2024 13:42:44 +0200 Subject: [PATCH] Reland "TaskQueueStdlib: Stop spamming on idle." MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 9463095df81b737a4be6bf10d14dab679457dd7b. Reason for revert: Problem fixed. Original change's description: > Revert "TaskQueueStdlib: Stop spamming on idle." > > This reverts commit 2fbaa8e3a3ef7d7f26e82bc3325fdca33f60c1d3. > > Reason for revert: Breaks tests > > Original change's description: > > TaskQueueStdlib: Stop spamming on idle. > > > > When the stdlib task queue has no work for >= 3s on > > Android, it will emit bogus deadlock warnings. Fix > > this by ensuring we're not triggering this behavior. > > > > Fixed: b/364436627 > > Change-Id: I3fe02e13993cbb4a59d5669df06c4c293d237bc4 > > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/361721 > > Commit-Queue: Markus Handell > > Reviewed-by: Jonas Oreland > > Reviewed-by: Mirko Bonadei > > Cr-Commit-Position: refs/heads/main@{#42955} > > Change-Id: Ie80d86f6aa0cdff3bc5ee1d79fa4c8144009e33f > No-Presubmit: true > No-Tree-Checks: true > No-Try: true > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/361741 > Auto-Submit: Markus Handell > Reviewed-by: Björn Terelius > Commit-Queue: Mirko Bonadei > Reviewed-by: Mirko Bonadei > Bot-Commit: rubber-stamper@appspot.gserviceaccount.com > Cr-Commit-Position: refs/heads/main@{#42960} Fixed: b/364436627 Change-Id: I7baa8c5c169e5c63b28e1a5363c06e3d8d3f4c7c Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/361742 Reviewed-by: Mirko Bonadei Auto-Submit: Markus Handell Commit-Queue: Markus Handell Cr-Commit-Position: refs/heads/main@{#42963} --- rtc_base/BUILD.gn | 5 ++++ rtc_base/event.h | 4 +++- rtc_base/task_queue_stdlib.cc | 2 +- rtc_base/task_queue_stdlib_unittest.cc | 32 ++++++++++++++++++++++++++ 4 files changed, 41 insertions(+), 2 deletions(-) diff --git a/rtc_base/BUILD.gn b/rtc_base/BUILD.gn index bbe25fdd14..86a0e81f2b 100644 --- a/rtc_base/BUILD.gn +++ b/rtc_base/BUILD.gn @@ -739,8 +739,13 @@ if (rtc_include_tests) { sources = [ "task_queue_stdlib_unittest.cc" ] deps = [ ":gunit_helpers", + ":logging", + ":rtc_event", ":rtc_task_queue_stdlib", + "../api/task_queue", "../api/task_queue:task_queue_test", + "../api/units:time_delta", + "../system_wrappers", "../test:test_main", "../test:test_support", ] diff --git a/rtc_base/event.h b/rtc_base/event.h index 12f6a7dca2..78b3c16646 100644 --- a/rtc_base/event.h +++ b/rtc_base/event.h @@ -58,6 +58,8 @@ class Event { // TODO(bugs.webrtc.org/14366): Consider removing this redundant alias. static constexpr webrtc::TimeDelta kForever = webrtc::TimeDelta::PlusInfinity(); + static constexpr webrtc::TimeDelta kDefaultWarnDuration = + webrtc::TimeDelta::Seconds(3); Event(); Event(bool manual_reset, bool initially_signaled); @@ -83,7 +85,7 @@ class Event { // Waits with the given timeout and a reasonable default warning timeout. bool Wait(webrtc::TimeDelta give_up_after) { return Wait(give_up_after, give_up_after.IsPlusInfinity() - ? webrtc::TimeDelta::Seconds(3) + ? kDefaultWarnDuration : kForever); } diff --git a/rtc_base/task_queue_stdlib.cc b/rtc_base/task_queue_stdlib.cc index 1ac01e1830..8b9e554cdb 100644 --- a/rtc_base/task_queue_stdlib.cc +++ b/rtc_base/task_queue_stdlib.cc @@ -249,7 +249,7 @@ void TaskQueueStdlib::ProcessTasks() { continue; } - flag_notify_.Wait(task.sleep_time); + flag_notify_.Wait(task.sleep_time, task.sleep_time); } // Ensure remaining deleted tasks are destroyed with Current() set up to this diff --git a/rtc_base/task_queue_stdlib_unittest.cc b/rtc_base/task_queue_stdlib_unittest.cc index 0654e9719c..97f0a2cbc6 100644 --- a/rtc_base/task_queue_stdlib_unittest.cc +++ b/rtc_base/task_queue_stdlib_unittest.cc @@ -10,7 +10,12 @@ #include "rtc_base/task_queue_stdlib.h" +#include "api/task_queue/task_queue_factory.h" #include "api/task_queue/task_queue_test.h" +#include "api/units/time_delta.h" +#include "rtc_base/event.h" +#include "rtc_base/logging.h" +#include "system_wrappers/include/sleep.h" #include "test/gtest.h" namespace webrtc { @@ -25,5 +30,32 @@ INSTANTIATE_TEST_SUITE_P(TaskQueueStdlib, TaskQueueTest, ::testing::Values(CreateTaskQueueFactory)); +class StringPtrLogSink : public rtc::LogSink { + public: + explicit StringPtrLogSink(std::string* log_data) : log_data_(log_data) {} + + private: + void OnLogMessage(const std::string& message) override { + OnLogMessage(absl::string_view(message)); + } + void OnLogMessage(absl::string_view message) override { + log_data_->append(message.begin(), message.end()); + } + std::string* const log_data_; +}; + +TEST(TaskQueueStdlib, AvoidsSpammingLogOnInactivity) { + std::string log_output; + StringPtrLogSink stream(&log_output); + rtc::LogMessage::AddLogToStream(&stream, rtc::LS_VERBOSE); + auto task_queue = CreateTaskQueueStdlibFactory()->CreateTaskQueue( + "test", TaskQueueFactory::Priority::NORMAL); + auto wait_duration = rtc::Event::kDefaultWarnDuration + TimeDelta::Seconds(1); + SleepMs(wait_duration.ms()); + EXPECT_EQ(log_output.length(), 0u); + task_queue = nullptr; + rtc::LogMessage::RemoveLogToStream(&stream); +} + } // namespace } // namespace webrtc