From bfb47a0ae49c7986e2ccf396fcb6fbb52c7ac398 Mon Sep 17 00:00:00 2001 From: Geoffrey McRae Date: Fri, 9 Jul 2021 04:50:14 +1000 Subject: [PATCH] [common] windows: update event fast path to use atomics Due to a failure to understand atomics when this code was originally written it has a critical flaw with the fast path where an event could be signalled when it should not be. This change set corrects this issue by using atomic operations. --- common/src/platform/windows/event.c | 58 ++++++++++++++++------------- 1 file changed, 33 insertions(+), 25 deletions(-) diff --git a/common/src/platform/windows/event.c b/common/src/platform/windows/event.c index f6dec460..ef6b36cf 100644 --- a/common/src/platform/windows/event.c +++ b/common/src/platform/windows/event.c @@ -23,6 +23,7 @@ #include "common/time.h" #include +#include struct LGEvent { @@ -31,7 +32,7 @@ struct LGEvent HANDLE handle; bool wrapped; unsigned int msSpinTime; - volatile bool signaled; + _Atomic(bool) signaled; }; LGEvent * lgCreateEvent(bool autoReset, unsigned int msSpinTime) @@ -48,7 +49,7 @@ LGEvent * lgCreateEvent(bool autoReset, unsigned int msSpinTime) event->handle = CreateEvent(NULL, autoReset ? FALSE : TRUE, FALSE, NULL); event->wrapped = false; event->msSpinTime = msSpinTime; - event->signaled = false; + atomic_store(&event->signaled, false); if (!event->handle) { @@ -74,7 +75,7 @@ LGEvent * lgWrapEvent(void * handle) event->handle = (HANDLE)handle; event->wrapped = true; event->msSpinTime = 0; - event->signaled = false; + atomic_store(&event->signaled, false); return event; } @@ -88,20 +89,19 @@ bool lgWaitEvent(LGEvent * event, unsigned int timeout) // wrapped events can't be enahnced if (!event->wrapped) { - if (event->signaled) + if (event->reset) { - if (event->reset) - event->signaled = false; - return true; + if (atomic_exchange(&event->signaled, false)) + return true; + } + else + { + if (atomic_load(&event->signaled)) + return true; } if (timeout == 0) - { - bool ret = event->signaled; - if (event->reset) - event->signaled = false; - return ret; - } + return false; if (event->msSpinTime) { @@ -119,17 +119,25 @@ bool lgWaitEvent(LGEvent * event, unsigned int timeout) uint64_t now = microtime(); uint64_t end = now + spinTime * 1000; - while(!event->signaled) - { - now = microtime(); - if (now >= end) - break; - } - if (event->signaled) + if (event->reset) { - if (event->reset) - event->signaled = false; + while(!atomic_exchange(&event->signaled, false)) + { + now = microtime(); + if (now >= end) + return false; + } + return true; + } + else + { + while(!atomic_load(&event->signaled)) + { + now = microtime(); + if (now >= end) + return false; + } return true; } } @@ -142,7 +150,7 @@ bool lgWaitEvent(LGEvent * event, unsigned int timeout) { case WAIT_OBJECT_0: if (!event->reset) - event->signaled = true; + atomic_store(&event->signaled, true); return true; case WAIT_ABANDONED: @@ -211,12 +219,12 @@ bool lgWaitEvents(LGEvent * events[], int count, bool waitAll, unsigned int time bool lgSignalEvent(LGEvent * event) { - event->signaled = true; + atomic_store(&event->signaled, true); return SetEvent(event->handle); } bool lgResetEvent(LGEvent * event) { - event->signaled = false; + atomic_store(&event->signaled, false); return ResetEvent(event->handle); }