From a4550f55ea2bcbac75fe3ee48ce08f700a41d4ab Mon Sep 17 00:00:00 2001
From: SChernykh <sergey.v.chernykh@gmail.com>
Date: Fri, 9 Oct 2020 10:29:18 +0200
Subject: [PATCH] Fix possible race condition in hashrate counting code

Use single atomic operation to switch between data points.
---
 src/backend/common/Worker.cpp           | 25 ++++++++++++++++++++-----
 src/backend/common/Worker.h             |  8 ++++----
 src/backend/common/Workers.cpp          |  8 ++++----
 src/backend/common/interfaces/IWorker.h | 15 +++++++--------
 4 files changed, 35 insertions(+), 21 deletions(-)

diff --git a/src/backend/common/Worker.cpp b/src/backend/common/Worker.cpp
index d6e01560..d2269300 100644
--- a/src/backend/common/Worker.cpp
+++ b/src/backend/common/Worker.cpp
@@ -32,9 +32,7 @@
 
 xmrig::Worker::Worker(size_t id, int64_t affinity, int priority) :
     m_affinity(affinity),
-    m_id(id),
-    m_hashCount(0),
-    m_timestamp(0)
+    m_id(id)
 {
     m_node = VirtualMemory::bindToNUMANode(affinity);
 
@@ -45,6 +43,23 @@ xmrig::Worker::Worker(size_t id, int64_t affinity, int priority) :
 
 void xmrig::Worker::storeStats()
 {
-    m_hashCount.store(m_count, std::memory_order_relaxed);
-    m_timestamp.store(Chrono::highResolutionMSecs(), std::memory_order_relaxed);
+    // Get next index which is unused now
+    const uint32_t index = (m_index.load(std::memory_order_relaxed) + 1) & 1;
+
+    // Fill in the data for that index
+    m_hashCount[index] = m_count;
+    m_timestamp[index] = Chrono::highResolutionMSecs();
+
+    // Switch to that index
+    // All data will be in memory by the time it completes thanks to std::memory_order_seq_cst
+    m_index.fetch_add(1, std::memory_order_seq_cst);
+}
+
+
+void xmrig::Worker::getHashrateData(uint64_t& hashCount, uint64_t& timeStamp) const
+{
+    const uint32_t index = m_index.load(std::memory_order_relaxed) & 1;
+
+    hashCount = m_hashCount[index];
+    timeStamp = m_timestamp[index];
 }
diff --git a/src/backend/common/Worker.h b/src/backend/common/Worker.h
index 3e87de1d..35f949ce 100644
--- a/src/backend/common/Worker.h
+++ b/src/backend/common/Worker.h
@@ -44,8 +44,7 @@ public:
 
     inline const VirtualMemory *memory() const override   { return nullptr; }
     inline size_t id() const override                     { return m_id; }
-    inline uint64_t hashCount() const override            { return m_hashCount.load(std::memory_order_relaxed); }
-    inline uint64_t timestamp() const override            { return m_timestamp.load(std::memory_order_relaxed); }
+    void getHashrateData(uint64_t& hashCount, uint64_t& timeStamp) const override;
     inline void jobEarlyNotification(const Job&) override {}
 
 protected:
@@ -53,8 +52,9 @@ protected:
 
     const int64_t m_affinity;
     const size_t m_id;
-    std::atomic<uint64_t> m_hashCount;
-    std::atomic<uint64_t> m_timestamp;
+    uint64_t m_hashCount[2] = {};
+    uint64_t m_timestamp[2] = {};
+    std::atomic<uint32_t> m_index;
     uint32_t m_node     = 0;
     uint64_t m_count    = 0;
 };
diff --git a/src/backend/common/Workers.cpp b/src/backend/common/Workers.cpp
index 1efa6cbe..56c4bd1a 100644
--- a/src/backend/common/Workers.cpp
+++ b/src/backend/common/Workers.cpp
@@ -143,11 +143,11 @@ void xmrig::Workers<T>::tick(uint64_t)
     }
 
     for (Thread<T> *handle : m_workers) {
-        if (!handle->worker()) {
-            continue;
+        if (handle->worker()) {
+            uint64_t hashCount, timeStamp;
+            handle->worker()->getHashrateData(hashCount, timeStamp);
+            d_ptr->hashrate->add(handle->id(), hashCount, timeStamp);
         }
-
-        d_ptr->hashrate->add(handle->id(), handle->worker()->hashCount(), handle->worker()->timestamp());
     }
 }
 
diff --git a/src/backend/common/interfaces/IWorker.h b/src/backend/common/interfaces/IWorker.h
index b74c308b..9a0e6f41 100644
--- a/src/backend/common/interfaces/IWorker.h
+++ b/src/backend/common/interfaces/IWorker.h
@@ -42,14 +42,13 @@ class IWorker
 public:
     virtual ~IWorker() = default;
 
-    virtual bool selfTest()                         = 0;
-    virtual const VirtualMemory *memory() const     = 0;
-    virtual size_t id() const                       = 0;
-    virtual size_t intensity() const                = 0;
-    virtual uint64_t hashCount() const              = 0;
-    virtual uint64_t timestamp() const              = 0;
-    virtual void start()                            = 0;
-    virtual void jobEarlyNotification(const Job&)   = 0;
+    virtual bool selfTest()                                   = 0;
+    virtual const VirtualMemory *memory() const               = 0;
+    virtual size_t id() const                                 = 0;
+    virtual size_t intensity() const                          = 0;
+    virtual void getHashrateData(uint64_t&, uint64_t&) const  = 0;
+    virtual void start()                                      = 0;
+    virtual void jobEarlyNotification(const Job&)             = 0;
 };