Add a DCHECK for PlatformThread instances that are too busy.
This adds a simple mechanism that provides protection against
implementations that use the legacy callback type in PlatformThread
and are prone to entering a busy loop.
Enabled only in DCHECK enabled builds.
BUG=webrtc:7187
Review-Url: https://codereview.webrtc.org/2720223003
Cr-Commit-Position: refs/heads/master@{#16973}
diff --git a/webrtc/base/platform_thread.cc b/webrtc/base/platform_thread.cc
index 52f8810..afacfdc 100644
--- a/webrtc/base/platform_thread.cc
+++ b/webrtc/base/platform_thread.cc
@@ -12,6 +12,7 @@
#include "webrtc/base/atomicops.h"
#include "webrtc/base/checks.h"
+#include "webrtc/base/timeutils.h"
#if defined(WEBRTC_LINUX)
#include <sys/prctl.h>
@@ -220,25 +221,53 @@
run_function_(obj_);
return;
}
-// TODO(tommi): Delete the below.
-#if !defined(WEBRTC_MAC) && !defined(WEBRTC_WIN)
- const struct timespec ts_null = {0};
+
+// TODO(tommi): Delete the rest of this function when looping isn't supported.
+#if RTC_DCHECK_IS_ON
+ // These constants control the busy loop detection algorithm below.
+ // |kMaxLoopCount| controls the limit for how many times we allow the loop
+ // to run within a period, before DCHECKing.
+ // |kPeriodToMeasureMs| controls how long that period is.
+ static const int kMaxLoopCount = 1000;
+ static const int kPeriodToMeasureMs = 100;
+ int64_t loop_stamps[kMaxLoopCount] = {};
+ int64_t sequence_nr = 0;
#endif
+
do {
// The interface contract of Start/Stop is that for a successful call to
// Start, there should be at least one call to the run function. So we
// call the function before checking |stop_|.
if (!run_function_deprecated_(obj_))
break;
+#if RTC_DCHECK_IS_ON
+ auto id = sequence_nr % kMaxLoopCount;
+ loop_stamps[id] = rtc::TimeMillis();
+ if (sequence_nr > kMaxLoopCount) {
+ auto compare_id = (id + 1) % kMaxLoopCount;
+ auto diff = loop_stamps[id] - loop_stamps[compare_id];
+ RTC_DCHECK_GE(diff, 0);
+ if (diff < kPeriodToMeasureMs) {
+ RTC_NOTREACHED() << "This thread is too busy: " << name_ << " " << diff
+ << "ms sequence=" << sequence_nr << " "
+ << loop_stamps[id] << " vs " << loop_stamps[compare_id]
+ << ", " << id << " vs " << compare_id;
+ }
+ }
+ ++sequence_nr;
+#endif
#if defined(WEBRTC_WIN)
// Alertable sleep to permit RaiseFlag to run and update |stop_|.
SleepEx(0, true);
} while (!stop_);
#else
-#if defined(WEBRTC_MAC)
- sched_yield();
-#else
+#if defined(UNDEFINED_SANITIZER) || defined(WEBRTC_ANDROID)
+ // UBSAN and Android don't like |sched_yield()| that much.
+ static const struct timespec ts_null = {0};
nanosleep(&ts_null, nullptr);
+#else // !(defined(UNDEFINED_SANITIZER) || defined(WEBRTC_ANDROID))
+ // Mac and Linux show better performance with sched_yield.
+ sched_yield();
#endif
} while (!AtomicOps::AcquireLoad(&stop_flag_));
#endif // defined(WEBRTC_WIN)
diff --git a/webrtc/base/platform_thread_unittest.cc b/webrtc/base/platform_thread_unittest.cc
index d6d35e4..415b9eb 100644
--- a/webrtc/base/platform_thread_unittest.cc
+++ b/webrtc/base/platform_thread_unittest.cc
@@ -17,7 +17,12 @@
namespace {
// Function that does nothing, and reports success.
bool NullRunFunctionDeprecated(void* obj) {
- webrtc::SleepMs(0); // Hand over timeslice, prevents busy looping.
+ webrtc::SleepMs(2); // Hand over timeslice, prevents busy looping.
+ return true;
+}
+
+bool TooBusyRunFunction(void* obj) {
+ // Indentionally busy looping.
return true;
}
@@ -108,4 +113,16 @@
EXPECT_TRUE(flag);
}
+// This test is disabled since it will cause a crash.
+// There might be a way to implement this as a death test, but it looks like
+// a death test requires an expression to be checked but does not allow a
+// flag to be raised that says "some thread will crash after this point".
+// TODO(tommi): Look into ways to enable the test by default.
+TEST(PlatformThreadTest, DISABLED_TooBusyDeprecated) {
+ PlatformThread thread(&TooBusyRunFunction, nullptr, "BusyThread");
+ thread.Start();
+ webrtc::SleepMs(1000);
+ thread.Stop();
+}
+
} // rtc
