From 7ffb5bb8285dd4cf3efacca9e478a468deaf9e89 Mon Sep 17 00:00:00 2001 From: xuwei06 Date: Wed, 28 Sep 2016 14:57:13 -0700 Subject: [PATCH] Fix potential dead lock in PyDataProvider2 This bug occasionally causes dead lock in test_RecurrentGradientMachine In general, conditional_variable::notify should be used together with mutex for changing condition. --- paddle/gserver/dataproviders/PyDataProvider2.cpp | 15 +++++++++------ paddle/utils/Logging.h | 2 +- paddle/utils/Util.h | 11 +++++++++++ 3 files changed, 21 insertions(+), 7 deletions(-) diff --git a/paddle/gserver/dataproviders/PyDataProvider2.cpp b/paddle/gserver/dataproviders/PyDataProvider2.cpp index 0b41f6a02aecc..4e0d910e909fc 100644 --- a/paddle/gserver/dataproviders/PyDataProvider2.cpp +++ b/paddle/gserver/dataproviders/PyDataProvider2.cpp @@ -377,11 +377,17 @@ class PyDataProvider2 : public DataProvider { std::swap(callingContexts_[cid], callingContexts_[0]); cid = 0; } + + PyObjectPtr front; + { + std::unique_lock l(mtx_); + front = pop_get_front(callingContexts_); + this->pullCV_.notify_all(); + } { PyGuard g; - callingContexts_.pop_front(); + front.reset(); } - this->pullCV_.notify_all(); continue; } } @@ -410,9 +416,6 @@ class PyDataProvider2 : public DataProvider { std::lock_guard guard(mtx_); poolActualSize_ += additionalBatchSize; dataPool_.emplace_back(data); - } - - { pullCV_.notify_all(); } } @@ -592,8 +595,8 @@ class PyDataProvider2 : public DataProvider { { std::lock_guard g(mtx_); poolActualSize_ -= bsize; + this->pushCV_.notify_all(); } - this->pushCV_.notify_all(); } if (bsize == 0) { // end of pass. In data pool, cannot get any data. diff --git a/paddle/utils/Logging.h b/paddle/utils/Logging.h index 7fdfa3240c1de..b3f439804686f 100644 --- 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 index 11a03e141dec5..57839f2e21573 100644 --- 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::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))) /**