diff --git a/src/main/cpp/appenderattachableimpl.cpp b/src/main/cpp/appenderattachableimpl.cpp index 73d673554..304b7061c 100644 --- a/src/main/cpp/appenderattachableimpl.cpp +++ b/src/main/cpp/appenderattachableimpl.cpp @@ -23,11 +23,45 @@ using namespace LOG4CXX_NS::helpers; IMPLEMENT_LOG4CXX_OBJECT(AppenderAttachableImpl) +using AppenderListPtr = std::shared_ptr; + +/** A vector of appender pointers. */ struct AppenderAttachableImpl::priv_data { - /** Array of appenders. */ - AppenderList appenderList; +private: // Attributes +#ifdef __cpp_lib_atomic_shared_ptr + std::atomic pAppenderList; +#else // !defined(__cpp_lib_atomic_shared_ptr) + AppenderListPtr pAppenderList; mutable std::mutex m_mutex; +#endif // !defined(__cpp_lib_atomic_shared_ptr) + +public: // ...structors + priv_data(const AppenderList& newList = {}) + : pAppenderList{ std::make_shared(newList) } + {} + +public: // Accessors + AppenderListPtr getAppenders() const + { +#ifdef __cpp_lib_atomic_shared_ptr + return pAppenderList.load(std::memory_order_acquire); +#else // !defined(__cpp_lib_atomic_shared_ptr) + std::lock_guard lock( m_mutex ); + return pAppenderList; +#endif // !defined(__cpp_lib_atomic_shared_ptr) + } + +public: // Modifiers + void setAppenders(const AppenderList& newList) + { +#ifdef __cpp_lib_atomic_shared_ptr + pAppenderList.store(std::make_shared(newList), std::memory_order_release); +#else // !defined(__cpp_lib_atomic_shared_ptr) + std::lock_guard lock( m_mutex ); + pAppenderList = std::make_shared(newList); +#endif // !defined(__cpp_lib_atomic_shared_ptr) + } }; AppenderAttachableImpl::AppenderAttachableImpl() @@ -36,76 +70,62 @@ AppenderAttachableImpl::AppenderAttachableImpl() #if LOG4CXX_ABI_VERSION <= 15 AppenderAttachableImpl::AppenderAttachableImpl(Pool& pool) - : m_priv() { } #endif - AppenderAttachableImpl::~AppenderAttachableImpl() { - } + void AppenderAttachableImpl::addAppender(const AppenderPtr newAppender) { - // Null values for newAppender parameter are strictly forbidden. if (!newAppender) - { return; - } - if (!m_priv) - m_priv = std::make_unique(); - - std::lock_guard lock( m_priv->m_mutex ); - AppenderList::iterator it = std::find( - m_priv->appenderList.begin(), m_priv->appenderList.end(), newAppender); - - if (it == m_priv->appenderList.end()) + if (m_priv) { - m_priv->appenderList.push_back(newAppender); + auto allAppenders = m_priv->getAppenders(); + if (allAppenders->end() == std::find(allAppenders->begin(), allAppenders->end(), newAppender)) + { + auto newAppenders = *allAppenders; + newAppenders.push_back(newAppender); + m_priv->setAppenders(newAppenders); + } } + else + m_priv = std::make_unique(AppenderList{newAppender}); } -int AppenderAttachableImpl::appendLoopOnAppenders( - const spi::LoggingEventPtr& event, - Pool& p) +int AppenderAttachableImpl::appendLoopOnAppenders(const spi::LoggingEventPtr& event, Pool& p) { - int numberAppended = 0; + int result = 0; if (m_priv) { - // FallbackErrorHandler::error() may modify our list of appenders - // while we are iterating over them (if it holds the same logger). - // So, make a local copy of the appenders that we want to iterate over - // before actually iterating over them. - AppenderList allAppenders = getAllAppenders(); - for (auto appender : allAppenders) + auto allAppenders = m_priv->getAppenders(); + for (auto& appender : *allAppenders) { appender->doAppend(event, p); - numberAppended++; + ++result; } } - - return numberAppended; + return result; } AppenderList AppenderAttachableImpl::getAllAppenders() const { AppenderList result; if (m_priv) - { - std::lock_guard lock( m_priv->m_mutex ); - result = m_priv->appenderList; - } + result = *m_priv->getAppenders(); return result; } AppenderPtr AppenderAttachableImpl::getAppender(const LogString& name) const { AppenderPtr result; - if (m_priv && !name.empty()) + if (m_priv) { - std::lock_guard lock( m_priv->m_mutex ); - for (auto appender : m_priv->appenderList) + auto allAppenders = m_priv->getAppenders(); + for (auto& appender : *allAppenders) { if (name == appender->getName()) { @@ -122,8 +142,8 @@ bool AppenderAttachableImpl::isAttached(const AppenderPtr appender) const bool result = false; if (m_priv && appender) { - std::lock_guard lock( m_priv->m_mutex ); - result = std::find(m_priv->appenderList.begin(), m_priv->appenderList.end(), appender) != m_priv->appenderList.end(); + auto allAppenders = m_priv->getAppenders(); + result = allAppenders->end() != std::find(allAppenders->begin(), allAppenders->end(), appender); } return result; } @@ -132,10 +152,10 @@ void AppenderAttachableImpl::removeAllAppenders() { if (m_priv) { - for (auto a : getAllAppenders()) - a->close(); - std::lock_guard lock( m_priv->m_mutex ); - m_priv->appenderList.clear(); + auto allAppenders = m_priv->getAppenders(); + for (auto& appender : *allAppenders) + appender->close(); + m_priv->setAppenders({}); } } @@ -143,27 +163,31 @@ void AppenderAttachableImpl::removeAppender(const AppenderPtr appender) { if (m_priv && appender) { - std::lock_guard lock( m_priv->m_mutex ); - auto it = std::find(m_priv->appenderList.begin(), m_priv->appenderList.end(), appender); - if (it != m_priv->appenderList.end()) + auto newAppenders = *m_priv->getAppenders(); + auto pItem = std::find(newAppenders.begin(), newAppenders.end(), appender); + if (newAppenders.end() != pItem) { - m_priv->appenderList.erase(it); + newAppenders.erase(pItem); + m_priv->setAppenders(newAppenders); } } } void AppenderAttachableImpl::removeAppender(const LogString& name) { - if (m_priv && !name.empty()) + if (m_priv) { - std::lock_guard lock( m_priv->m_mutex ); - auto it = std::find_if(m_priv->appenderList.begin(), m_priv->appenderList.end() + auto newAppenders = *m_priv->getAppenders(); + auto pItem = std::find_if(newAppenders.begin(), newAppenders.end() , [&name](const AppenderPtr& appender) -> bool { return name == appender->getName(); }); - if (it != m_priv->appenderList.end()) - m_priv->appenderList.erase(it); + if (newAppenders.end() != pItem) + { + newAppenders.erase(pItem); + m_priv->setAppenders(newAppenders); + } } } @@ -172,16 +196,17 @@ bool AppenderAttachableImpl::replaceAppender(const AppenderPtr& oldAppender, con bool found = false; if (m_priv && oldAppender && newAppender) { - auto oldName = oldAppender->getName(); - std::lock_guard lock( m_priv->m_mutex ); - auto it = std::find_if(m_priv->appenderList.begin(), m_priv->appenderList.end() - , [&oldName](const AppenderPtr& appender) -> bool + auto name = oldAppender->getName(); + auto newAppenders = *m_priv->getAppenders(); + auto pItem = std::find_if(newAppenders.begin(), newAppenders.end() + , [&name](const AppenderPtr& appender) -> bool { - return oldName == appender->getName(); + return name == appender->getName(); }); - if (it != m_priv->appenderList.end()) + if (newAppenders.end() != pItem) { - *it = newAppender; + *pItem = newAppender; + m_priv->setAppenders(newAppenders); found = true; } } @@ -190,13 +215,15 @@ bool AppenderAttachableImpl::replaceAppender(const AppenderPtr& oldAppender, con void AppenderAttachableImpl::replaceAppenders(const AppenderList& newList) { - auto oldAppenders = getAllAppenders(); - if (!m_priv) - m_priv = std::make_unique(); - std::lock_guard lock( m_priv->m_mutex ); - for (auto a : oldAppenders) - a->close(); - m_priv->appenderList = newList; + if (m_priv) + { + auto allAppenders = m_priv->getAppenders(); + for (auto& a : *allAppenders) + a->close(); + m_priv->setAppenders(newList); + } + else + m_priv = std::make_unique(newList); }