Fix potential dead lock in PyDataProvider2 (#140)

This bug occasionally causes dead lock in test_RecurrentGradientMachine In general, conditional_variable::notify should be used together with mutex for changing condition.
PaddlePaddle · Sep 29, 2016 · 6decbdf · 6decbdf
1 parent 4615c51
commit 6decbdf
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 6 deletions.
diff --git a/paddle/gserver/dataproviders/PyDataProvider2.cpp b/paddle/gserver/dataproviders/PyDataProvider2.cpp
@@ -377,9 +377,15 @@ class PyDataProvider2 : public DataProvider {
             std::swap(callingContexts_[cid], callingContexts_[0]);
             cid = 0;
           }
+
+          PyObjectPtr front;
+          {
+            std::unique_lock<std::mutex> l(mtx_);
+            front = pop_get_front(callingContexts_);
+          }
           {
             PyGuard g;
-            callingContexts_.pop_front();
+            front.reset();
           }
           this->pullCV_.notify_all();
           continue;
@@ -411,10 +417,7 @@ class PyDataProvider2 : public DataProvider {
         poolActualSize_ += additionalBatchSize;
         dataPool_.emplace_back(data);
       }
-
-      {
-        pullCV_.notify_all();
-      }
+      pullCV_.notify_all();
     }
     DBG << "load thread end";
   }

diff --git a/paddle/utils/Logging.h b/paddle/utils/Logging.h
@@ -191,7 +191,7 @@ void installFailureWriter(void(*callback)(const char*, int));
 }
 #endif  // PADDLE_USE_GLOG
 
-#ifndef NDEBUG
+#ifdef NDEBUG
 #define DEBUG_LEVEL 5
 #define DBG VLOG(DEBUG_LEVEL)
 #else

diff --git a/paddle/utils/Util.h b/paddle/utils/Util.h
@@ -112,6 +112,17 @@ static bool contains(const Container& container, const T& val) {
   return std::find(container.begin(), container.end(), val) != container.end();
 }
 
+/**
+ * pop and get the front element of a container
+ */
+template<typename Container>
+typename Container::value_type pop_get_front(Container& c) {
+  typename Container::value_type v;
+  swap(v, c.front());
+  c.pop_front();
+  return v;
+}
+
 #define ARRAYSIZE(a) (sizeof(a) / sizeof(*(a)))
 
 /**