From 6decbdf4f53cf1733b2ffc8cfd29120010f4264e Mon Sep 17 00:00:00 2001 From: emailweixu Date: Wed, 28 Sep 2016 20:35:11 -0700 Subject: [PATCH] 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. --- paddle/gserver/dataproviders/PyDataProvider2.cpp | 13 ++++++++----- paddle/utils/Logging.h | 2 +- paddle/utils/Util.h | 11 +++++++++++ 3 files changed, 20 insertions(+), 6 deletions(-) diff --git a/paddle/gserver/dataproviders/PyDataProvider2.cpp b/paddle/gserver/dataproviders/PyDataProvider2.cpp index 0b41f6a02..2f9a1223c 100644 --- a/paddle/gserver/dataproviders/PyDataProvider2.cpp +++ b/paddle/gserver/dataproviders/PyDataProvider2.cpp @@ -377,9 +377,15 @@ private: std::swap(callingContexts_[cid], callingContexts_[0]); cid = 0; } + + PyObjectPtr front; + { + std::unique_lock l(mtx_); + front = pop_get_front(callingContexts_); + } { PyGuard g; - callingContexts_.pop_front(); + front.reset(); } this->pullCV_.notify_all(); continue; @@ -411,10 +417,7 @@ private: 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 index 7fdfa3240..b3f439804 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 11a03e141..57839f2e2 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))) /** -- GitLab