From f523197dfd8da7eba70599995656a5984f3fa00a Mon Sep 17 00:00:00 2001
From: Mathew Sutcliffe <oktal3700@gmail.com>
Date: Fri, 20 May 2016 01:25:05 +0100
Subject: [PATCH] refs #657 Fixed race condition in synchronize().

---
 src/blackmisc/datacache.cpp | 24 ++++++++++++++++++++++--
 src/blackmisc/datacache.h   |  3 +++
 2 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/src/blackmisc/datacache.cpp b/src/blackmisc/datacache.cpp
index eb664ddfa..c55033e28 100644
--- a/src/blackmisc/datacache.cpp
+++ b/src/blackmisc/datacache.cpp
@@ -28,6 +28,7 @@
 #include <Qt>
 #include <memory>
 #include <utility>
+#include <chrono>
 
 namespace BlackMisc
 {
@@ -107,10 +108,18 @@ namespace BlackMisc
 
     bool CDataCache::synchronize(const QString &key)
     {
+        constexpr auto timeout = std::chrono::seconds(1);
+        constexpr auto ready = std::future_status::ready;
+        constexpr auto zero = std::chrono::seconds::zero();
+
         auto future = m_revision.promiseLoadedValue(key, getTimestampSync(key));
         if (future.valid())
         {
-            future.wait();
+            std::future_status s {};
+            do { s = future.wait_for(timeout); } while (s != ready && m_revision.isNewerValueAvailable(key, getTimestampSync(key)));
+            if (s != ready) { s = future.wait_for(zero); }
+            if (s != ready) { return false; }
+            try { future.get(); } catch (const std::future_error &) { return false; } // broken promise
             return true;
         }
         return false;
@@ -285,7 +294,6 @@ namespace BlackMisc
 
         Q_ASSERT(! m_updateInProgress);
         Q_ASSERT(! m_lockFile.isLocked());
-        Q_ASSERT(m_promises.empty());
 
         if (! m_lockFile.lock())
         {
@@ -396,6 +404,7 @@ namespace BlackMisc
         m_updateInProgress = false;
         m_pendingRead = false;
         m_pendingWrite = false;
+        breakPromises();
         m_lockFile.unlock();
     }
 
@@ -451,6 +460,17 @@ namespace BlackMisc
         return std::move(m_promises); // move into the return value, so m_promises becomes empty
     }
 
+    void CDataCacheRevision::breakPromises()
+    {
+        QMutexLocker lock(&m_mutex);
+
+        if (! m_promises.empty())
+        {
+            CLogMessage(this).debug() << "Breaking" << m_promises.size() << "promises";
+            m_promises.clear();
+        }
+    }
+
     QString CDataCacheRevision::timestampsAsString() const
     {
         QMutexLocker lock(&m_mutex);
diff --git a/src/blackmisc/datacache.h b/src/blackmisc/datacache.h
index 0e7fff9f4..392169828 100644
--- a/src/blackmisc/datacache.h
+++ b/src/blackmisc/datacache.h
@@ -126,6 +126,9 @@ namespace BlackMisc
         //! Returns (by move) the container of promises to load values.
         std::vector<std::promise<void>> loadedValuePromises();
 
+        //! Abandon all promises.
+        void breakPromises();
+
         //! Keys with timestamps.
         QString timestampsAsString() const;