From 2fc37573b9a0a6dbabee66056d420737565c58ec Mon Sep 17 00:00:00 2001 From: Tommi Date: Mon, 24 Oct 2022 09:22:16 +0200 Subject: [PATCH] Add DISALLOW_WAIT(), a regression catching utility. Bug: none Change-Id: I94def80a7eb815f74d1be691d21b5f3661771a71 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/279821 Reviewed-by: Harald Alvestrand Reviewed-by: Evan Shrubsole Commit-Queue: Tomas Gunnarsson Cr-Commit-Position: refs/heads/main@{#38457} --- rtc_base/event.h | 45 ++++++++++++++++++++++++++++++++++++++ rtc_base/event_unittest.cc | 10 +++++++++ 2 files changed, 55 insertions(+) diff --git a/rtc_base/event.h b/rtc_base/event.h index 941497ca7b..12f6a7dca2 100644 --- a/rtc_base/event.h +++ b/rtc_base/event.h @@ -12,6 +12,7 @@ #define RTC_BASE_EVENT_H_ #include "api/units/time_delta.h" + #if defined(WEBRTC_WIN) #include #elif defined(WEBRTC_POSIX) @@ -20,8 +21,38 @@ #error "Must define either WEBRTC_WIN or WEBRTC_POSIX." #endif +#include "rtc_base/synchronization/yield_policy.h" + namespace rtc { +// RTC_DISALLOW_WAIT() utility +// +// Sets a stack-scoped flag that disallows use of `rtc::Event::Wait` by means +// of raising a DCHECK when a call to `rtc::Event::Wait()` is made.. +// This is useful to guard synchronization-free scopes against regressions. +// +// Example of what this would catch (`ScopeToProtect` calls `Foo`): +// +// void Foo(TaskQueue* tq) { +// Event event; +// tq->PostTask([&event]() { +// event.Set(); +// }); +// event.Wait(Event::kForever); // <- Will trigger a DCHECK. +// } +// +// void ScopeToProtect() { +// TaskQueue* tq = GetSomeTaskQueue(); +// RTC_DISALLOW_WAIT(); // Policy takes effect. +// Foo(tq); +// } +// +#if RTC_DCHECK_IS_ON +#define RTC_DISALLOW_WAIT() ScopedDisallowWait disallow_wait_##__LINE__ +#else +#define RTC_DISALLOW_WAIT() +#endif + class Event { public: // TODO(bugs.webrtc.org/14366): Consider removing this redundant alias. @@ -87,6 +118,20 @@ class ScopedAllowBaseSyncPrimitivesForTesting { ~ScopedAllowBaseSyncPrimitivesForTesting() {} }; +#if RTC_DCHECK_IS_ON +class ScopedDisallowWait { + public: + ScopedDisallowWait() = default; + + private: + class DisallowYieldHandler : public YieldInterface { + public: + void YieldExecution() override { RTC_DCHECK_NOTREACHED(); } + } handler_; + rtc::ScopedYieldPolicy policy{&handler_}; +}; +#endif + } // namespace rtc #endif // RTC_BASE_EVENT_H_ diff --git a/rtc_base/event_unittest.cc b/rtc_base/event_unittest.cc index e35373142e..17f50dc2d1 100644 --- a/rtc_base/event_unittest.cc +++ b/rtc_base/event_unittest.cc @@ -102,4 +102,14 @@ TEST(EventTest, DISABLED_PerformanceMultiThread) { thread.Stop(); } +#if RTC_DCHECK_IS_ON && GTEST_HAS_DEATH_TEST && !defined(WEBRTC_ANDROID) +// Tests that we crash if we attempt to call rtc::Event::Wait while we're +// not allowed to (as per `RTC_DISALLOW_WAIT()`). +TEST(EventTestDeathTest, DisallowEventWait) { + Event event; + RTC_DISALLOW_WAIT(); + EXPECT_DEATH(event.Wait(Event::kForever), ""); +} +#endif // RTC_DCHECK_IS_ON && GTEST_HAS_DEATH_TEST && !defined(WEBRTC_ANDROID) + } // namespace rtc