Index: base/synchronization/waitable_event_win.cc |
diff --git a/base/synchronization/waitable_event_win.cc b/base/synchronization/waitable_event_win.cc |
index d80cabb3ff3ae864fb25f54448885d2c252c5f2b..df274aab39773dc12334a8c704b17dedfde2272a 100644 |
--- a/base/synchronization/waitable_event_win.cc |
+++ b/base/synchronization/waitable_event_win.cc |
@@ -7,6 +7,7 @@ |
#include <windows.h> |
#include <stddef.h> |
+#include <algorithm> |
#include <utility> |
#include "base/debug/activity_tracker.h" |
@@ -44,7 +45,10 @@ void WaitableEvent::Signal() { |
} |
bool WaitableEvent::IsSignaled() { |
- return TimedWait(TimeDelta()); |
+ DWORD result = WaitForSingleObject(handle_.Get(), 0); |
+ DCHECK(result == WAIT_OBJECT_0 || result == WAIT_TIMEOUT) |
+ << "Unexpected WaitForSingleObject result " << result; |
+ return result == WAIT_OBJECT_0; |
} |
void WaitableEvent::Wait() { |
@@ -58,30 +62,64 @@ void WaitableEvent::Wait() { |
DCHECK_EQ(WAIT_OBJECT_0, result) << "WaitForSingleObject failed"; |
} |
-bool WaitableEvent::TimedWait(const TimeDelta& max_time) { |
+namespace { |
+// Helper function called from TimedWait and TimedWaitUntil. |
+bool WaitUntil(HANDLE handle, const TimeTicks& now, const TimeTicks& end_time) { |
+ TimeDelta delta = end_time - now; |
+ DCHECK_GT(delta, TimeDelta()); |
+ |
+ do { |
+ // On Windows, waiting for less than 1 ms results in WaitForSingleObject |
+ // returning promptly which may result in the caller code spinning. |
+ // We need to ensure that we specify at least the minimally possible 1 ms |
+ // delay unless the initial timeout was exactly zero. |
+ delta = std::max(delta, TimeDelta::FromMilliseconds(1)); |
+ // Truncate the timeout to milliseconds. |
+ DWORD timeout_ms = saturated_cast<DWORD>(delta.InMilliseconds()); |
+ DWORD result = WaitForSingleObject(handle, timeout_ms); |
+ DCHECK(result == WAIT_OBJECT_0 || result == WAIT_TIMEOUT) |
+ << "Unexpected WaitForSingleObject result " << result; |
+ switch (result) { |
+ case WAIT_OBJECT_0: |
+ return true; |
+ case WAIT_TIMEOUT: |
+ // TimedWait can time out earlier than the specified |timeout| on |
+ // Windows. To make this consistent with the posix implementation we |
+ // should guarantee that TimedWait doesn't return earlier than the |
+ // specified |max_time| and wait again for the remaining time. |
+ delta = end_time - TimeTicks::Now(); |
+ break; |
+ } |
+ } while (delta > TimeDelta()); |
+ return false; |
+} |
+} // namespace |
+ |
+bool WaitableEvent::TimedWait(const TimeDelta& max_delta) { |
// Record the event that this thread is blocking upon (for hang diagnosis). |
base::debug::ScopedEventWaitActivity event_activity(this); |
+ base::ThreadRestrictions::AssertWaitAllowed(); |
danakj
2016/11/16 22:57:29
This should be after the check for is_zero right?
stanisc
2016/11/17 19:42:22
That's what I had before, but I thought you sugges
|
- DCHECK_GE(max_time, TimeDelta()); |
- if (!max_time.is_zero()) |
- base::ThreadRestrictions::AssertWaitAllowed(); |
- |
- // Truncate the timeout to milliseconds. The API specifies that this method |
- // can return in less than |max_time| (when returning false), as the argument |
- // is the maximum time that a caller is willing to wait. |
- DWORD timeout = saturated_cast<DWORD>(max_time.InMilliseconds()); |
- |
- DWORD result = WaitForSingleObject(handle_.Get(), timeout); |
- switch (result) { |
- case WAIT_OBJECT_0: |
- return true; |
- case WAIT_TIMEOUT: |
- return false; |
- } |
- // It is most unexpected that this should ever fail. Help consumers learn |
- // about it if it should ever fail. |
- NOTREACHED() << "WaitForSingleObject failed"; |
- return false; |
+ DCHECK_GE(max_delta, TimeDelta()); |
+ if (max_delta.is_zero()) |
+ return IsSignaled(); |
+ |
+ TimeTicks now(TimeTicks::Now()); |
+ // TimeTicks takes care of overflow including the cases when max_delta |
+ // is a maximum value. |
+ return WaitUntil(handle_.Get(), now, now + max_delta); |
+} |
+ |
+bool WaitableEvent::TimedWaitUntil(const TimeTicks& end_time) { |
+ // Record the event that this thread is blocking upon (for hang diagnosis). |
+ base::debug::ScopedEventWaitActivity event_activity(this); |
+ base::ThreadRestrictions::AssertWaitAllowed(); |
danakj
2016/11/16 22:57:29
same, but we should check for is_zero explicitly h
stanisc
2016/11/17 19:42:22
Moved the assert below the check. Shouldn't zero T
danakj
2016/11/17 21:11:08
0 would be handled, but other things too. I don't
stanisc
2016/11/18 02:13:47
OK, I've added a special case for zero TimeTicks.
|
+ |
+ TimeTicks now(TimeTicks::Now()); |
+ if (end_time <= now) |
+ return IsSignaled(); |
+ |
+ return WaitUntil(handle_.Get(), now, end_time); |
} |
// static |