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..d0b13ee3d2a6a4cc327e3edc7c32a3e4d0e34db7 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,18 @@ void WaitableEvent::Signal() { |
} |
bool WaitableEvent::IsSignaled() { |
- return TimedWait(TimeDelta()); |
+ DWORD result = WaitForSingleObject(handle_.Get(), 0); |
+ 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; |
} |
void WaitableEvent::Wait() { |
@@ -59,28 +71,44 @@ void WaitableEvent::Wait() { |
} |
bool WaitableEvent::TimedWait(const TimeDelta& max_time) { |
danakj
2016/10/21 22:01:22
nit: can you name timedeltas with _delta suffix an
stanisc
2016/11/11 03:50:48
Done.
|
- // Record the event that this thread is blocking upon (for hang diagnosis). |
- base::debug::ScopedEventWaitActivity event_activity(this); |
- |
DCHECK_GE(max_time, TimeDelta()); |
- if (!max_time.is_zero()) |
- base::ThreadRestrictions::AssertWaitAllowed(); |
+ if (max_time.is_zero()) |
+ return IsSignaled(); |
- // 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()); |
+ // Record the event that this thread is blocking upon (for hang diagnosis). |
+ base::debug::ScopedEventWaitActivity event_activity(this); |
+ base::ThreadRestrictions::AssertWaitAllowed(); |
danakj
2016/10/21 22:01:22
can you put assertions first? (just above event_ac
stanisc
2016/11/11 03:50:48
Done.
|
- 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"; |
+ constexpr TimeDelta min_delay = TimeDelta::FromMilliseconds(1); |
+ TimeTicks target_time = TimeTicks::Now() + max_time; |
danakj
2016/10/21 22:01:22
It's possible this was a bad suggestion because it
stanisc
2016/10/21 23:05:25
Yeah, that is unfortunate and redundant that the c
danakj
2016/10/21 23:15:26
I was wondering about an overload also, this seems
|
+ TimeDelta delay = max_time; |
+ |
+ 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. |
+ delay = std::max(delay, min_delay); |
danakj
2016/10/21 22:01:22
I think you can just write TimeDelta::FromMillisec
stanisc
2016/11/11 03:50:48
Done.
|
+ // Truncate the timeout to milliseconds. |
+ DWORD timeout = saturated_cast<DWORD>(delay.InMilliseconds()); |
+ DWORD result = WaitForSingleObject(handle_.Get(), timeout); |
+ 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. |
+ delay = target_time - TimeTicks::Now(); |
+ break; |
+ default: |
+ // It is most unexpected that this should ever fail. Help consumers |
+ // learn about it if it should ever fail. |
+ NOTREACHED() << "WaitForSingleObject failed"; |
danakj
2016/10/21 22:01:22
This falls into the NOTREACHED+return antipattern.
stanisc
2016/11/11 03:50:48
Good suggestion. Fixed.
|
+ return false; |
+ } |
+ } while (delay > TimeDelta()); |
return false; |
} |