NullSocketServer::Wait: Don't warn if we have to wait a long time for messages
Make the warning timeout for Event::Wait configurable, and let
NullSocketServer::Wait pass kForever to completely eliminate the
warning.
3000 ms is a good default warning timeout for Event::Wait, but in some
cases---such as when a message queue is waiting for a message to
arrive---we don't want the warning, since a long wait isn't a reliable
indicator that the system is deadlocked. It might just be that no one
is posting messages.
Bug: webrtc:10531
Change-Id: Ic5969b8bfedb96376bd6d6a72ba6a4591750a920
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/132017
Reviewed-by: Magnus Jedvert <magjed@webrtc.org>
Commit-Queue: Karl Wiberg <kwiberg@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#27574}diff --git a/rtc_base/BUILD.gn b/rtc_base/BUILD.gn
index 6a2c5aa..05cecd1 100644
--- a/rtc_base/BUILD.gn
+++ b/rtc_base/BUILD.gn
@@ -265,6 +265,7 @@
":checks",
"synchronization:yield_policy",
"system:warn_current_thread_is_deadlocked",
+ "//third_party/abseil-cpp/absl/types:optional",
]
}
}
diff --git a/rtc_base/event.cc b/rtc_base/event.cc
index 4b8e9ff..67c8746 100644
--- a/rtc_base/event.cc
+++ b/rtc_base/event.cc
@@ -21,6 +21,7 @@
#error "Must define either WEBRTC_WIN or WEBRTC_POSIX."
#endif
+#include "absl/types/optional.h"
#include "rtc_base/checks.h"
#include "rtc_base/synchronization/yield_policy.h"
#include "rtc_base/system/warn_current_thread_is_deadlocked.h"
@@ -50,9 +51,9 @@
ResetEvent(event_handle_);
}
-bool Event::Wait(int milliseconds) {
+bool Event::Wait(const int give_up_after_ms, int /*warn_after_ms*/) {
ScopedYieldPolicy::YieldExecution();
- DWORD ms = (milliseconds == kForever) ? INFINITE : milliseconds;
+ const DWORD ms = give_up_after_ms == kForever ? INFINITE : give_up_after_ms;
return (WaitForSingleObject(event_handle_, ms) == WAIT_OBJECT_0);
}
@@ -135,34 +136,54 @@
} // namespace
-bool Event::Wait(const int milliseconds) {
- // Set a timeout for the given number of milliseconds, or 3000 ms if the
- // caller asked for kForever.
- const timespec ts =
- GetTimespec(milliseconds == kForever ? 3000 : milliseconds);
+bool Event::Wait(const int give_up_after_ms, const int warn_after_ms) {
+ // Instant when we'll log a warning message (because we've been waiting so
+ // long it might be a bug), but not yet give up waiting. nullopt if we
+ // shouldn't log a warning.
+ const absl::optional<timespec> warn_ts =
+ warn_after_ms == kForever ||
+ (give_up_after_ms != kForever && warn_after_ms > give_up_after_ms)
+ ? absl::nullopt
+ : absl::make_optional(GetTimespec(warn_after_ms));
+
+ // Instant when we'll stop waiting and return an error. nullopt if we should
+ // never give up.
+ const absl::optional<timespec> give_up_ts =
+ give_up_after_ms == kForever
+ ? absl::nullopt
+ : absl::make_optional(GetTimespec(give_up_after_ms));
ScopedYieldPolicy::YieldExecution();
pthread_mutex_lock(&event_mutex_);
- // Wait for the event to trigger or the timeout to expire, whichever comes
- // first.
- int error = 0;
- while (!event_status_ && error == 0) {
-#if USE_PTHREAD_COND_TIMEDWAIT_MONOTONIC_NP
- error =
- pthread_cond_timedwait_monotonic_np(&event_cond_, &event_mutex_, &ts);
-#else
- error = pthread_cond_timedwait(&event_cond_, &event_mutex_, &ts);
-#endif
- }
-
- if (milliseconds == kForever && error == ETIMEDOUT) {
- // Our 3000 ms timeout expired, but the caller asked us to wait forever, so
- // do that.
- webrtc::WarnThatTheCurrentThreadIsProbablyDeadlocked();
- error = 0;
+ // Wait for `event_cond_` to trigger and `event_status_` to be set, with the
+ // given timeout (or without a timeout if none is given).
+ const auto wait = [&](const absl::optional<timespec> timeout_ts) {
+ int error = 0;
while (!event_status_ && error == 0) {
- error = pthread_cond_wait(&event_cond_, &event_mutex_);
+ if (timeout_ts == absl::nullopt) {
+ error = pthread_cond_wait(&event_cond_, &event_mutex_);
+ } else {
+#if USE_PTHREAD_COND_TIMEDWAIT_MONOTONIC_NP
+ error = pthread_cond_timedwait_monotonic_np(&event_cond_, &event_mutex_,
+ &*timeout_ts);
+#else
+ error =
+ pthread_cond_timedwait(&event_cond_, &event_mutex_, &*timeout_ts);
+#endif
+ }
+ }
+ return error;
+ };
+
+ int error;
+ if (warn_ts == absl::nullopt) {
+ error = wait(give_up_ts);
+ } else {
+ error = wait(warn_ts);
+ if (error == ETIMEDOUT) {
+ webrtc::WarnThatTheCurrentThreadIsProbablyDeadlocked();
+ error = wait(give_up_ts);
}
}
diff --git a/rtc_base/event.h b/rtc_base/event.h
index 3f5f8b3..584ad5d 100644
--- a/rtc_base/event.h
+++ b/rtc_base/event.h
@@ -34,9 +34,21 @@
void Set();
void Reset();
- // Wait for the event to become signaled, for the specified number of
- // |milliseconds|. To wait indefinetly, pass kForever.
- bool Wait(int milliseconds);
+ // Waits for the event to become signaled, but logs a warning if it takes more
+ // than `warn_after_ms` milliseconds, and gives up completely if it takes more
+ // than `give_up_after_ms` milliseconds. (If `warn_after_ms >=
+ // give_up_after_ms`, no warning will be logged.) Either or both may be
+ // `kForever`, which means wait indefinitely.
+ //
+ // Returns true if the event was signaled, false if there was a timeout or
+ // some other error.
+ bool Wait(int give_up_after_ms, int warn_after_ms);
+
+ // Waits with the given timeout and a reasonable default warning timeout.
+ bool Wait(int give_up_after_ms) {
+ return Wait(give_up_after_ms,
+ give_up_after_ms == kForever ? 3000 : kForever);
+ }
private:
#if defined(WEBRTC_WIN)
diff --git a/rtc_base/null_socket_server.cc b/rtc_base/null_socket_server.cc
index f446b3d..b2071e3 100644
--- a/rtc_base/null_socket_server.cc
+++ b/rtc_base/null_socket_server.cc
@@ -17,7 +17,10 @@
NullSocketServer::~NullSocketServer() {}
bool NullSocketServer::Wait(int cms, bool process_io) {
- event_.Wait(cms);
+ // Wait with the given timeout. Do not log a warning if we end up waiting for
+ // a long time; that just means no one has any work for us, which is perfectly
+ // legitimate.
+ event_.Wait(/*give_up_after_ms=*/cms, /*warn_after_ms=*/Event::kForever);
return true;
}
