Merge pull request #1881 from SChernykh/dev

Fix possible race condition in hashrate counting code

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 index which is unused now
+    const uint32_t index = m_index.load(std::memory_order_relaxed) ^ 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_xor(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);
+
+    hashCount = m_hashCount[index];
+    timeStamp = m_timestamp[index];
 }

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;
 };

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());
     }
 }
 

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;
 };