Use atomic<bool> primitive for thread's shared flags.

4 files changed, 68 insertions, 47 deletions

diff --git a/search/search_engine.cpp b/search/search_engine.cpp
index 737f5ee98f..830b60a011 100644
--- a/search/search_engine.cpp
+++ b/search/search_engine.cpp
@@ -186,26 +186,26 @@ void Engine::SetViewportAsync(m2::RectD const & viewport, m2::RectD const & near
 
 void Engine::SearchAsync()
 {
+  // Avoid many threads waiting in search mutex. One is enough.
+  bool expected = false;
+  if (!m_readyThread.compare_exchange_strong(expected, true))
   {
-    // Avoid many threads waiting in search mutex. One is enough.
-    threads::MutexGuard readyGuard(m_readyMutex);
-    if (m_readyThread)
-      return;
-    m_readyThread = true;
+    ASSERT ( expected, () );
+    return;
   }
 
-  // First of all - cancel previous query.
+  // Order of next 4 operations is very important!
+
+  // 1. Cancel previous query.
   m_pQuery->DoCancel();
 
-  // Enter to run new search.
+  // 2. Enter to run new search.
   threads::MutexGuard searchGuard(m_searchMutex);
 
-  {
-    threads::MutexGuard readyGuard(m_readyMutex);
-    m_readyThread = false;
-  }
+  // 3. Free flag for the next search thread.
+  m_readyThread.store(false);
 
-  // Get current search params.
+  // 4. Get current search params.
   SearchParams params;
   m2::RectD arrRects[2];
 
diff --git a/search/search_engine.hpp b/search/search_engine.hpp
index 7a20b703af..043871c49f 100644
--- a/search/search_engine.hpp
+++ b/search/search_engine.hpp
@@ -11,6 +11,7 @@
 #include "../std/scoped_ptr.hpp"
 #include "../std/string.hpp"
 #include "../std/function.hpp"
+#include "../std/atomic.hpp"
 
 
 class Index;
@@ -61,9 +62,8 @@ private:
   void SetViewportAsync(m2::RectD const & viewport, m2::RectD const & nearby);
   void SearchAsync();
 
-  threads::Mutex m_searchMutex, m_updateMutex, m_readyMutex;
-
-  volatile bool m_readyThread;
+  threads::Mutex m_searchMutex, m_updateMutex;
+  volatile atomic<bool> m_readyThread;
 
   SearchParams m_params;
   m2::RectD m_viewport;
diff --git a/search/search_query.cpp b/search/search_query.cpp
index c529e06fdc..11fadfc1d1 100644
--- a/search/search_query.cpp
+++ b/search/search_query.cpp
@@ -131,7 +131,8 @@ namespace
 
 void Query::SetViewport(m2::RectD viewport[], size_t count)
 {
-  ASSERT_LESS_OR_EQUAL(count, static_cast<size_t>(RECTSCOUNT), ());
+  // use static_cast to avoid GCC linker dummy bug
+  ASSERT_LESS ( count, static_cast<size_t>(RECTSCOUNT), () );
 
   m_cancel = false;
 
@@ -144,7 +145,8 @@ void Query::SetViewport(m2::RectD viewport[], size_t count)
 
 void Query::SetViewportByIndex(MWMVectorT const & mwmInfo, m2::RectD const & viewport, size_t idx)
 {
-  ASSERT_LESS_OR_EQUAL(idx, static_cast<size_t>(RECTSCOUNT), ());
+  // use static_cast to avoid GCC linker dummy bug
+  ASSERT_LESS ( idx, static_cast<size_t>(RECTSCOUNT), () );
 
   if (viewport.IsValid())
   {
@@ -292,7 +294,6 @@ void Query::Search(string const & query, Results & res)
   SearchAddress();
 
   if (m_cancel) return;
-  LOG(LDEBUG, ("Usual search"));
   SearchFeatures();
 
   if (m_cancel) return;
@@ -1335,9 +1336,10 @@ namespace
   {
     vector<uint32_t> const * m_offsets;
 
-    volatile bool & m_isCancelled;
+    volatile atomic<bool> const & m_isCancelled;
   public:
-    FeaturesFilter(vector<uint32_t> const * offsets, volatile bool & isCancelled)
+    FeaturesFilter(vector<uint32_t> const * offsets,
+                   volatile atomic<bool> const & isCancelled)
       : m_offsets(offsets), m_isCancelled(isCancelled)
     {
     }
@@ -1345,7 +1347,11 @@ namespace
     bool operator() (uint32_t offset) const
     {
       if (m_isCancelled)
+      {
+        //LOG(LINFO, ("Throw CancelException"));
+        //dbg::ObjectTracker::PrintLeaks();
         throw Query::CancelException();
+      }
 
       return (m_offsets == 0 ||
               binary_search(m_offsets->begin(), m_offsets->end(), offset));
@@ -1362,7 +1368,11 @@ namespace
     TrieValuesHolder(FilterT const & filter) : m_filter(filter) {}
 
     void Resize(size_t count) { m_holder.resize(count); }
-    void StartNew(size_t ind) { m_ind = ind; }
+    void StartNew(size_t ind)
+    {
+      ASSERT_LESS ( ind, m_holder.size(), () );
+      m_ind = ind;
+    }
     void operator() (Query::TrieValueT const & v)
     {
       if (m_filter(v.m_featureId))
@@ -1390,6 +1400,38 @@ void Query::SearchFeatures(Params const & params, MWMVectorT const & mwmInfo, in
   }
 }
 
+namespace
+{
+
+void FillCategories(Query::Params const & params, TrieIterator const * pTrieRoot,
+                    TrieValuesHolder<FeaturesFilter> & categoriesHolder)
+{
+  scoped_ptr<TrieIterator> pCategoriesRoot;
+  typedef TrieIterator::Edge::EdgeStrT EdgeT;
+  EdgeT categoriesEdge;
+
+  size_t const count = pTrieRoot->m_edge.size();
+  for (size_t i = 0; i < count; ++i)
+  {
+    EdgeT const & edge = pTrieRoot->m_edge[i].m_str;
+    ASSERT_GREATER_OR_EQUAL(edge.size(), 1, ());
+
+    if (edge[0] == search::CATEGORIES_LANG)
+    {
+      categoriesEdge = edge;
+      pCategoriesRoot.reset(pTrieRoot->GoToEdge(i));
+      break;
+    }
+  }
+  ASSERT_NOT_EQUAL(pCategoriesRoot, 0, ());
+
+  GetFeaturesInTrie(params.m_tokens, params.m_prefixTokens,
+                    TrieRootPrefix(*pCategoriesRoot, categoriesEdge),
+                    categoriesHolder);
+}
+
+}
+
 void Query::SearchInMWM(Index::MwmLock const & mwmLock, Params const & params, int ind/* = -1*/)
 {
   if (MwmValue * pMwm = mwmLock.GetValue())
@@ -1416,33 +1458,11 @@ void Query::SearchInMWM(Index::MwmLock const & mwmLock, Params const & params, i
 
       // Get categories for each token separately - find needed edge with categories.
       TrieValuesHolder<FeaturesFilter> categoriesHolder(filter);
-      size_t const count = pTrieRoot->m_edge.size();
-      {
-        scoped_ptr<TrieIterator> pCategoriesRoot;
-        TrieIterator::Edge::EdgeStrT categoriesEdge;
-
-        for (size_t i = 0; i < count; ++i)
-        {
-          TrieIterator::Edge::EdgeStrT const & edge = pTrieRoot->m_edge[i].m_str;
-          ASSERT_GREATER_OR_EQUAL(edge.size(), 1, ());
-
-          if (edge[0] == search::CATEGORIES_LANG)
-          {
-            categoriesEdge = edge;
-            pCategoriesRoot.reset(pTrieRoot->GoToEdge(i));
-            break;
-          }
-        }
-        ASSERT_NOT_EQUAL(pCategoriesRoot, 0, ());
-
-        GetFeaturesInTrie(params.m_tokens, params.m_prefixTokens,
-                          TrieRootPrefix(*pCategoriesRoot, categoriesEdge),
-                          categoriesHolder);
-      }
-
-      impl::FeatureLoader emitter(*this, mwmId, ind);
+      FillCategories(params, pTrieRoot.get(), categoriesHolder);
 
       // Match tokens to feature for each language - iterate through first edges.
+      impl::FeatureLoader emitter(*this, mwmId, ind);
+      size_t const count = pTrieRoot->m_edge.size();
       for (size_t i = 0; i < count; ++i)
       {
         TrieIterator::Edge::EdgeStrT const & edge = pTrieRoot->m_edge[i].m_str;
diff --git a/search/search_query.hpp b/search/search_query.hpp
index 575fd66567..f1d5862dcb 100644
--- a/search/search_query.hpp
+++ b/search/search_query.hpp
@@ -16,6 +16,7 @@
 #include "../std/string.hpp"
 #include "../std/unordered_set.hpp"
 #include "../std/vector.hpp"
+#include "../std/atomic.hpp"
 
 
 class FeatureType;
@@ -163,7 +164,7 @@ private:
   StringsToSuggestVectorT const * m_pStringsToSuggest;
   storage::CountryInfoGetter const * m_pInfoGetter;
 
-  volatile bool m_cancel;
+  volatile atomic<bool> m_cancel;
 
   buffer_vector<strings::UniString, 32> m_tokens;
   strings::UniString m_prefix;