[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.
This commit is contained in:
Geoffrey McRae 2021-07-09 04:50:14 +10:00
parent 59efa6f0ad
commit bfb47a0ae4

View File

@ -23,6 +23,7 @@
#include "common/time.h" #include "common/time.h"
#include <windows.h> #include <windows.h>
#include <stdatomic.h>
struct LGEvent struct LGEvent
{ {
@ -31,7 +32,7 @@ struct LGEvent
HANDLE handle; HANDLE handle;
bool wrapped; bool wrapped;
unsigned int msSpinTime; unsigned int msSpinTime;
volatile bool signaled; _Atomic(bool) signaled;
}; };
LGEvent * lgCreateEvent(bool autoReset, unsigned int msSpinTime) 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->handle = CreateEvent(NULL, autoReset ? FALSE : TRUE, FALSE, NULL);
event->wrapped = false; event->wrapped = false;
event->msSpinTime = msSpinTime; event->msSpinTime = msSpinTime;
event->signaled = false; atomic_store(&event->signaled, false);
if (!event->handle) if (!event->handle)
{ {
@ -74,7 +75,7 @@ LGEvent * lgWrapEvent(void * handle)
event->handle = (HANDLE)handle; event->handle = (HANDLE)handle;
event->wrapped = true; event->wrapped = true;
event->msSpinTime = 0; event->msSpinTime = 0;
event->signaled = false; atomic_store(&event->signaled, false);
return event; return event;
} }
@ -87,21 +88,20 @@ bool lgWaitEvent(LGEvent * event, unsigned int timeout)
{ {
// wrapped events can't be enahnced // wrapped events can't be enahnced
if (!event->wrapped) if (!event->wrapped)
{
if (event->signaled)
{ {
if (event->reset) if (event->reset)
event->signaled = false; {
if (atomic_exchange(&event->signaled, false))
return true;
}
else
{
if (atomic_load(&event->signaled))
return true; return true;
} }
if (timeout == 0) if (timeout == 0)
{ return false;
bool ret = event->signaled;
if (event->reset)
event->signaled = false;
return ret;
}
if (event->msSpinTime) if (event->msSpinTime)
{ {
@ -119,17 +119,25 @@ bool lgWaitEvent(LGEvent * event, unsigned int timeout)
uint64_t now = microtime(); uint64_t now = microtime();
uint64_t end = now + spinTime * 1000; uint64_t end = now + spinTime * 1000;
while(!event->signaled)
if (event->reset)
{
while(!atomic_exchange(&event->signaled, false))
{ {
now = microtime(); now = microtime();
if (now >= end) if (now >= end)
break; return false;
} }
return true;
if (event->signaled) }
else
{ {
if (event->reset) while(!atomic_load(&event->signaled))
event->signaled = false; {
now = microtime();
if (now >= end)
return false;
}
return true; return true;
} }
} }
@ -142,7 +150,7 @@ bool lgWaitEvent(LGEvent * event, unsigned int timeout)
{ {
case WAIT_OBJECT_0: case WAIT_OBJECT_0:
if (!event->reset) if (!event->reset)
event->signaled = true; atomic_store(&event->signaled, true);
return true; return true;
case WAIT_ABANDONED: case WAIT_ABANDONED:
@ -211,12 +219,12 @@ bool lgWaitEvents(LGEvent * events[], int count, bool waitAll, unsigned int time
bool lgSignalEvent(LGEvent * event) bool lgSignalEvent(LGEvent * event)
{ {
event->signaled = true; atomic_store(&event->signaled, true);
return SetEvent(event->handle); return SetEvent(event->handle);
} }
bool lgResetEvent(LGEvent * event) bool lgResetEvent(LGEvent * event)
{ {
event->signaled = false; atomic_store(&event->signaled, false);
return ResetEvent(event->handle); return ResetEvent(event->handle);
} }