Fix potential data race in mutex lock implementation.
Writing or reading an ordinary volatile boolean does not provide memory
ordering guarantees. Use atomic<bool> with release and acquire semantics
instead. On x86 this should have no impact on the generated code, since
it architecturally offers strong memory ordering guarantees.
Bug chromium:710753
Change-Id: I5aa2a6eeecfcd691a0aa8d06fd067f9d59ab13c3
Reviewed-on: https://swiftshader-review.googlesource.com/9328
Reviewed-by: Corentin Wallez <cwallez@google.com>
Reviewed-by: Nicolas Capens <capn@google.com>
Tested-by: Nicolas Capens <capn@google.com>
diff --git a/src/Common/MutexLock.hpp b/src/Common/MutexLock.hpp
index 044c156..d9f4034 100644
--- a/src/Common/MutexLock.hpp
+++ b/src/Common/MutexLock.hpp
@@ -59,6 +59,8 @@
#else // !__ANDROID__
+#include <atomic>
+
namespace sw
{
class BackoffLock
@@ -73,7 +75,7 @@
{
if(!isLocked())
{
- if(atomicExchange(&mutex, 1) == 0)
+ if(mutex.exchange(true) == false)
{
return true;
}
@@ -148,12 +150,12 @@
void unlock()
{
- mutex = 0;
+ mutex.store(false, std::memory_order_release);
}
bool isLocked()
{
- return mutex != 0;
+ return mutex.load(std::memory_order_acquire);
}
private:
@@ -162,7 +164,7 @@
// Ensure that the mutex variable is on its own 64-byte cache line to avoid false sharing
// Padding must be public to avoid compiler warnings
volatile int padding1[16];
- volatile int mutex;
+ std::atomic<bool> mutex;
volatile int padding2[15];
};
};
