Commit dc7759cc by Nicolas Capens

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/9328Reviewed-by: 's avatarCorentin Wallez <cwallez@google.com> Reviewed-by: 's avatarNicolas Capens <capn@google.com> Tested-by: 's avatarNicolas Capens <capn@google.com>
parent ad675fa1
...@@ -59,6 +59,8 @@ namespace sw ...@@ -59,6 +59,8 @@ namespace sw
#else // !__ANDROID__ #else // !__ANDROID__
#include <atomic>
namespace sw namespace sw
{ {
class BackoffLock class BackoffLock
...@@ -73,7 +75,7 @@ namespace sw ...@@ -73,7 +75,7 @@ namespace sw
{ {
if(!isLocked()) if(!isLocked())
{ {
if(atomicExchange(&mutex, 1) == 0) if(mutex.exchange(true) == false)
{ {
return true; return true;
} }
...@@ -148,12 +150,12 @@ namespace sw ...@@ -148,12 +150,12 @@ namespace sw
void unlock() void unlock()
{ {
mutex = 0; mutex.store(false, std::memory_order_release);
} }
bool isLocked() bool isLocked()
{ {
return mutex != 0; return mutex.load(std::memory_order_acquire);
} }
private: private:
...@@ -162,7 +164,7 @@ namespace sw ...@@ -162,7 +164,7 @@ namespace sw
// Ensure that the mutex variable is on its own 64-byte cache line to avoid false sharing // 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 // Padding must be public to avoid compiler warnings
volatile int padding1[16]; volatile int padding1[16];
volatile int mutex; std::atomic<bool> mutex;
volatile int padding2[15]; volatile int padding2[15];
}; };
}; };
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment