From 19ecd43340ce16ced6be530c9dff5cf724a6e160 Mon Sep 17 00:00:00 2001 From: Josh Blum Date: Wed, 1 Jun 2011 13:57:25 -0700 Subject: uhd: tweaks to log and msg implementation The implementations now contain the string stream in each instance. This way there is not a global stringstream to lock access to. This resolves the issue of nested log calls locking condition. --- host/include/uhd/utils/log.hpp | 6 +++++- host/include/uhd/utils/msg.hpp | 6 +++++- host/lib/utils/log.cpp | 42 +++++++++++++++++++----------------------- host/lib/utils/msg.cpp | 21 +++++++++++---------- 4 files changed, 40 insertions(+), 35 deletions(-) diff --git a/host/include/uhd/utils/log.hpp b/host/include/uhd/utils/log.hpp index bc8f41fb4..1f515c15e 100644 --- a/host/include/uhd/utils/log.hpp +++ b/host/include/uhd/utils/log.hpp @@ -19,6 +19,7 @@ #define INCLUDED_UHD_UTILS_LOG_HPP #include +#include #include #include #include @@ -77,7 +78,8 @@ namespace uhd{ namespace _log{ }; //! Internal logging object (called by UHD_LOG macros) - struct UHD_API log{ + class UHD_API log{ + public: log( const verbosity_t verbosity, const std::string &file, @@ -86,6 +88,8 @@ namespace uhd{ namespace _log{ ); ~log(void); std::ostream &operator()(void); + private: + UHD_PIMPL_DECL(impl) _impl; }; }} //namespace uhd::_log diff --git a/host/include/uhd/utils/msg.hpp b/host/include/uhd/utils/msg.hpp index 050504e57..71d2cb35e 100644 --- a/host/include/uhd/utils/msg.hpp +++ b/host/include/uhd/utils/msg.hpp @@ -19,6 +19,7 @@ #define INCLUDED_UHD_UTILS_MSG_HPP #include +#include #include #include @@ -52,10 +53,13 @@ namespace uhd{ namespace msg{ UHD_API void register_handler(const handler_t &handler); //! Internal message object (called by UHD_MSG macro) - struct UHD_API _msg{ + class UHD_API _msg{ + public: _msg(const type_t type); ~_msg(void); std::ostream &operator()(void); + private: + UHD_PIMPL_DECL(impl) _impl; }; }} //namespace uhd::msg diff --git a/host/lib/utils/log.cpp b/host/lib/utils/log.cpp index 7d11877a1..31d11796e 100644 --- a/host/lib/utils/log.cpp +++ b/host/lib/utils/log.cpp @@ -90,9 +90,7 @@ static fs::path get_temp_path(void){ **********************************************************************/ class log_resource_type{ public: - boost::mutex mutex; - std::ostringstream ss; - uhd::_log::verbosity_t verbosity, level; + uhd::_log::verbosity_t level; log_resource_type(void){ @@ -113,19 +111,20 @@ public: } ~log_resource_type(void){ + boost::mutex::scoped_lock lock(_mutex); _file_stream.close(); if (_file_lock != NULL) delete _file_lock; } - void log_to_file(void){ - if (verbosity < level) return; + void log_to_file(const std::string &log_msg){ + boost::mutex::scoped_lock lock(_mutex); if (_file_lock == NULL){ const std::string log_path = (get_temp_path() / "uhd.log").string(); _file_stream.open(log_path.c_str(), std::fstream::out | std::fstream::app); _file_lock = new ip::file_lock(log_path.c_str()); } _file_lock->lock(); - _file_stream << ss.str() << std::flush; + _file_stream << log_msg << std::flush; _file_lock->unlock(); } @@ -149,6 +148,7 @@ private: //file stream and lock: std::ofstream _file_stream; ip::file_lock *_file_lock; + boost::mutex _mutex; }; UHD_SINGLETON_FCN(log_resource_type, log_rs); @@ -167,20 +167,25 @@ static std::string get_rel_file_path(const fs::path &file){ return rel_path.string(); } +struct uhd::_log::log::impl{ + std::ostringstream ss; + verbosity_t verbosity; +}; + uhd::_log::log::log( const verbosity_t verbosity, const std::string &file, const unsigned int line, const std::string &function ){ - log_rs().mutex.lock(); - log_rs().verbosity = verbosity; + _impl = UHD_PIMPL_MAKE(impl, ()); + _impl->verbosity = verbosity; const std::string time = pt::to_simple_string(pt::microsec_clock::local_time()); const std::string header1 = str(boost::format("-- %s - level %d") % time % int(verbosity)); const std::string header2 = str(boost::format("-- %s") % function).substr(0, 80); const std::string header3 = str(boost::format("-- %s:%u") % get_rel_file_path(file) % line); const std::string border = std::string(std::max(std::max(header1.size(), header2.size()), header3.size()), '-'); - log_rs().ss + _impl->ss << std::endl << border << std::endl << header1 << std::endl @@ -191,35 +196,26 @@ uhd::_log::log::log( } uhd::_log::log::~log(void){ - log_rs().ss << std::endl; + if (_impl->verbosity < log_rs().level) return; + _impl->ss << std::endl; try{ - log_rs().log_to_file(); + log_rs().log_to_file(_impl->ss.str()); } catch(const std::exception &e){ /*! * Critical behavior below. * The following steps must happen in order to avoid a lock-up condition. * This is because the message facility will call into the logging facility. - * Therefore we must disable the logger (level = never) and unlock the mutex. - * - * 1) Set logging level to never. - * 2) Unlock the mutex. - * 3) Send the error message. - * 4) Return. + * Therefore we must disable the logger (level = never) before messaging. */ log_rs().level = never; - log_rs().ss.str(""); //clear for next call - log_rs().mutex.unlock(); UHD_MSG(error) << "Logging failed: " << e.what() << std::endl << "Logging has been disabled for this process" << std::endl ; - return; } - log_rs().ss.str(""); //clear for next call - log_rs().mutex.unlock(); } std::ostream & uhd::_log::log::operator()(void){ - return log_rs().ss; + return _impl->ss; } diff --git a/host/lib/utils/msg.cpp b/host/lib/utils/msg.cpp index 0fd62bfc1..de98ada64 100644 --- a/host/lib/utils/msg.cpp +++ b/host/lib/utils/msg.cpp @@ -65,8 +65,6 @@ static void msg_to_cerr(const std::string &title, const std::string &msg){ **********************************************************************/ struct msg_resource_type{ boost::mutex mutex; - std::ostringstream ss; - uhd::msg::type_t type; uhd::msg::handler_t handler; }; @@ -76,9 +74,8 @@ UHD_SINGLETON_FCN(msg_resource_type, msg_rs); * Setup the message handlers **********************************************************************/ void uhd::msg::register_handler(const handler_t &handler){ - msg_rs().mutex.lock(); + boost::mutex::scoped_lock lock(msg_rs().mutex); msg_rs().handler = handler; - msg_rs().mutex.unlock(); } static void default_msg_handler(uhd::msg::type_t type, const std::string &msg){ @@ -111,17 +108,21 @@ UHD_STATIC_BLOCK(msg_register_default_handler){ /*********************************************************************** * The message object implementation **********************************************************************/ +struct uhd::msg::_msg::impl{ + std::ostringstream ss; + type_t type; +}; + uhd::msg::_msg::_msg(const type_t type){ - msg_rs().mutex.lock(); - msg_rs().type = type; + _impl = UHD_PIMPL_MAKE(impl, ()); + _impl->type = type; } uhd::msg::_msg::~_msg(void){ - msg_rs().handler(msg_rs().type, msg_rs().ss.str()); - msg_rs().ss.str(""); //clear for next call - msg_rs().mutex.unlock(); + boost::mutex::scoped_lock lock(msg_rs().mutex); + msg_rs().handler(_impl->type, _impl->ss.str()); } std::ostream & uhd::msg::_msg::operator()(void){ - return msg_rs().ss; + return _impl->ss; } -- cgit v1.2.3