fix handle spurious wakeup wrongly returning Flush too early (#340)

add an atomic bool to check if flush has actually happened and condition variable did not spuriously wakeup.

fixes #339

Author: hendrikmuhs

keyvi/include/keyvi/index/internal/index_writer_worker.h

@@ -211,17 +211,20 @@ class IndexWriterWorker final {
     } else {
       std::condition_variable c;
       std::unique_lock<std::mutex> lock(payload_.flush_mutex_);
+      std::atomic_bool flushed{false};
 
-      compiler_active_object_([&c](IndexPayload& payload) {
-        {
-          PersistDeletes(&payload);
-          Compile(&payload);
-          std::unique_lock<std::mutex> lock(payload.flush_mutex_);
-        }
+      compiler_active_object_([&c, &flushed](IndexPayload& payload) {
+        std::unique_lock<std::mutex> lock(payload.flush_mutex_);
+        PersistDeletes(&payload);
+        Compile(&payload);
+        flushed = true;
         c.notify_all();
       });
 
-      c.wait(lock);
+      // condition may be unblocked spuriously, check flushed
+      while (flushed == false) {
+        c.wait(lock);
+      }
     }
   }
 

keyvi/tests/keyvi/index/index_limits_test.cpp

@@ -37,7 +37,7 @@
 #include "keyvi/index/index.h"
 
 inline std::string get_keyvimerger_bin() {
-  boost::filesystem::path path{std::getenv("KEYVI_UNITTEST_BASEPATH")};
+  boost::filesystem::path path{std::getenv("KEYVI_UNITTEST_BASEPATH")};  // NOLINT
   path /= DEFAULT_KEYVIMERGER_BIN;
 
   BOOST_CHECK(boost::filesystem::is_regular_file(path));
@@ -46,10 +46,12 @@ inline std::string get_keyvimerger_bin() {
 }
 
 inline size_t limit_filedescriptors(size_t file_descriptor_limit) {
-  struct rlimit limit;
+  struct rlimit limit {
+    0
+  };
 
   getrlimit(RLIMIT_NOFILE, &limit);
-  size_t old_limit = limit.rlim_cur;
+  const size_t old_limit = limit.rlim_cur;
   limit.rlim_cur = file_descriptor_limit;
   BOOST_CHECK(setrlimit(RLIMIT_NOFILE, &limit) == 0);
   getrlimit(RLIMIT_NOFILE, &limit);
@@ -65,7 +67,7 @@ BOOST_AUTO_TEST_CASE(filedescriptor_limit) {
   using boost::filesystem::temp_directory_path;
   using boost::filesystem::unique_path;
 
-  size_t old_limit = limit_filedescriptors(40);
+  const size_t old_limit = limit_filedescriptors(40);
 
   auto tmp_path = temp_directory_path();
   tmp_path /= unique_path("index-limits-test-temp-index-%%%%-%%%%-%%%%-%%%%");
@@ -80,13 +82,13 @@ BOOST_AUTO_TEST_CASE(filedescriptor_limit) {
     }
     writer.Flush();
     BOOST_CHECK(writer.Contains("a"));
-    dictionary::match_t m = writer["a"];
+    const dictionary::match_t m = writer["a"];
 
     BOOST_CHECK_EQUAL("{\"id\":4999}", m->GetValueAsString());
   }
   boost::filesystem::remove_all(tmp_path);
 
-  size_t increased_file_descriptors = keyvi::util::OsUtils::TryIncreaseFileDescriptors();
+  const size_t increased_file_descriptors = keyvi::util::OsUtils::TryIncreaseFileDescriptors();
   BOOST_CHECK(increased_file_descriptors > 40);
 
   limit_filedescriptors(old_limit);