This is an automated email from the ASF dual-hosted git repository.
rmiddleton pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/logging-log4cxx.git
The following commit(s) were added to refs/heads/master by this push:
new 674f43d LOGCXX-523 Call error handler if rollover fails (#64)
674f43d is described below
commit 674f43db0dce625afe6ec9981afeb1fda6ab923b
Author: Robert Middleton <[email protected]>
AuthorDate: Mon Apr 26 20:29:39 2021 -0400
LOGCXX-523 Call error handler if rollover fails (#64)
* LOGCXX-523 Call error handler if rollover fails
If the rollover fails, call the specified error handler for
the given appender. This means that you can now replace appenders
with a new appender.
The entire API at this point is not very robust at the moment. A
better error handling mechanism needs to be made to properly
handle the errors so that the error handler can have enough
data to do its job.
* Fix build on Windows
* Add include file needed for windows
---
src/main/cpp/appenderattachableimpl.cpp | 25 ++++++++--
src/main/cpp/asyncappender.cpp | 8 ----
src/main/cpp/logger.cpp | 81 ++++-----------------------------
src/main/cpp/rollingfileappender.cpp | 21 +++++++--
4 files changed, 47 insertions(+), 88 deletions(-)
diff --git a/src/main/cpp/appenderattachableimpl.cpp
b/src/main/cpp/appenderattachableimpl.cpp
index 6e63488..df077a1 100644
--- a/src/main/cpp/appenderattachableimpl.cpp
+++ b/src/main/cpp/appenderattachableimpl.cpp
@@ -42,6 +42,7 @@ void AppenderAttachableImpl::addAppender(const AppenderPtr
newAppender)
return;
}
+ std::unique_lock<std::mutex> lock( m_mutex );
AppenderList::iterator it = std::find(
appenderList.begin(), appenderList.end(), newAppender);
@@ -55,14 +56,27 @@ int AppenderAttachableImpl::appendLoopOnAppenders(
const spi::LoggingEventPtr& event,
Pool& p)
{
- for (AppenderList::iterator it = appenderList.begin();
- it != appenderList.end();
+
+ AppenderList allAppenders;
+ int numberAppended = 0;
+ {
+ // There are occasions where we want to modify our list of
appenders
+ // while we are iterating over them. For example, if one of the
+ // appenders fails, we may want to swap it out for a new one.
+ // So, make a local copy of the appenders that we want to
iterate over
+ // before actually iterating over them.
+ std::unique_lock<std::mutex> lock( m_mutex );
+ allAppenders = appenderList;
+ }
+ for (AppenderList::iterator it = allAppenders.begin();
+ it != allAppenders.end();
it++)
{
(*it)->doAppend(event, p);
+ numberAppended++;
}
- return (int)appenderList.size();
+ return numberAppended;
}
AppenderList AppenderAttachableImpl::getAllAppenders() const
@@ -77,6 +91,7 @@ AppenderPtr AppenderAttachableImpl::getAppender(const
LogString& name) const
return 0;
}
+ std::unique_lock<std::mutex> lock( m_mutex );
AppenderList::const_iterator it, itEnd = appenderList.end();
AppenderPtr appender;
@@ -100,6 +115,7 @@ bool AppenderAttachableImpl::isAttached(const AppenderPtr
appender) const
return false;
}
+ std::unique_lock<std::mutex> lock( m_mutex );
AppenderList::const_iterator it = std::find(
appenderList.begin(), appenderList.end(), appender);
@@ -108,6 +124,7 @@ bool AppenderAttachableImpl::isAttached(const AppenderPtr
appender) const
void AppenderAttachableImpl::removeAllAppenders()
{
+ std::unique_lock<std::mutex> lock( m_mutex );
AppenderList::iterator it, itEnd = appenderList.end();
AppenderPtr a;
@@ -127,6 +144,7 @@ void AppenderAttachableImpl::removeAppender(const
AppenderPtr appender)
return;
}
+ std::unique_lock<std::mutex> lock( m_mutex );
AppenderList::iterator it = std::find(
appenderList.begin(), appenderList.end(), appender);
@@ -143,6 +161,7 @@ void AppenderAttachableImpl::removeAppender(const
LogString& name)
return;
}
+ std::unique_lock<std::mutex> lock( m_mutex );
AppenderList::iterator it, itEnd = appenderList.end();
AppenderPtr appender;
diff --git a/src/main/cpp/asyncappender.cpp b/src/main/cpp/asyncappender.cpp
index 89300bc..6da23b1 100644
--- a/src/main/cpp/asyncappender.cpp
+++ b/src/main/cpp/asyncappender.cpp
@@ -61,7 +61,6 @@ AsyncAppender::~AsyncAppender()
void AsyncAppender::addAppender(const AppenderPtr newAppender)
{
- std::unique_lock<std::mutex> lock(appenders->getMutex());
appenders->addAppender(newAppender);
}
@@ -221,19 +220,16 @@ void AsyncAppender::close()
AppenderList AsyncAppender::getAllAppenders() const
{
- std::unique_lock<std::mutex> lock(appenders->getMutex());
return appenders->getAllAppenders();
}
AppenderPtr AsyncAppender::getAppender(const LogString& n) const
{
- std::unique_lock<std::mutex> lock(appenders->getMutex());
return appenders->getAppender(n);
}
bool AsyncAppender::isAttached(const AppenderPtr appender) const
{
- std::unique_lock<std::mutex> lock(appenders->getMutex());
return appenders->isAttached(appender);
}
@@ -244,19 +240,16 @@ bool AsyncAppender::requiresLayout() const
void AsyncAppender::removeAllAppenders()
{
- std::unique_lock<std::mutex> lock(appenders->getMutex());
appenders->removeAllAppenders();
}
void AsyncAppender::removeAppender(const AppenderPtr appender)
{
- std::unique_lock<std::mutex> lock(appenders->getMutex());
appenders->removeAppender(appender);
}
void AsyncAppender::removeAppender(const LogString& n)
{
- std::unique_lock<std::mutex> lock(appenders->getMutex());
appenders->removeAppender(n);
}
@@ -403,7 +396,6 @@ void AsyncAppender::dispatch()
iter != events.end();
iter++)
{
- std::unique_lock<std::mutex>
lock(appenders->getMutex());
appenders->appendLoopOnAppenders(*iter, p);
}
}
diff --git a/src/main/cpp/logger.cpp b/src/main/cpp/logger.cpp
index 71c16ed..fc617ae 100644
--- a/src/main/cpp/logger.cpp
+++ b/src/main/cpp/logger.cpp
@@ -42,7 +42,7 @@ IMPLEMENT_LOG4CXX_OBJECT(Logger)
Logger::Logger(Pool& p, const LogString& name1)
: pool(&p), name(), level(), parent(), resourceBundle(),
- repository(), aai()
+ repository(), aai(new AppenderAttachableImpl(*pool))
{
name = name1;
additive = true;
@@ -55,17 +55,9 @@ Logger::~Logger()
void Logger::addAppender(const AppenderPtr newAppender)
{
log4cxx::spi::LoggerRepositoryPtr rep;
- {
- log4cxx::shared_lock<log4cxx::shared_mutex> lock(mutex);
-
- if (aai == 0)
- {
- aai = AppenderAttachableImplPtr(new
AppenderAttachableImpl(*pool));
- }
- aai->addAppender(newAppender);
- rep = repository.lock();
- }
+ aai->addAppender(newAppender);
+ rep = repository.lock();
if (rep)
{
@@ -79,13 +71,7 @@ void Logger::reconfigure( const std::vector<AppenderPtr>&
appenders, bool additi
additive = additive1;
- if (aai != 0)
- {
- aai->removeAllAppenders();
- aai = 0;
- }
-
- aai = AppenderAttachableImplPtr(new AppenderAttachableImpl(*pool));
+ aai->removeAllAppenders();
for ( std::vector<AppenderPtr>::const_iterator it = appenders.cbegin();
it != appenders.cend();
@@ -108,13 +94,7 @@ void Logger::callAppenders(const spi::LoggingEventPtr&
event, Pool& p) const
logger != 0;
logger = logger->parent.get())
{
- // Protected against simultaneous call to addAppender,
removeAppender,...
- std::unique_lock<log4cxx::shared_mutex> lock(logger->mutex);
-
- if (logger->aai != 0)
- {
- writes += logger->aai->appendLoopOnAppenders(event, p);
- }
+ writes += logger->aai->appendLoopOnAppenders(event, p);
if (!logger->additive)
{
@@ -175,27 +155,11 @@ bool Logger::getAdditivity() const
AppenderList Logger::getAllAppenders() const
{
- log4cxx::shared_lock<log4cxx::shared_mutex> lock(mutex);
-
- if (aai == 0)
- {
- return AppenderList();
- }
- else
- {
- return aai->getAllAppenders();
- }
+ return aai->getAllAppenders();
}
AppenderPtr Logger::getAppender(const LogString& name1) const
{
- log4cxx::shared_lock<log4cxx::shared_mutex> lock(mutex);
-
- if (aai == 0 || name1.empty())
- {
- return 0;
- }
-
return aai->getAppender(name1);
}
@@ -275,16 +239,7 @@ LevelPtr Logger::getLevel() const
bool Logger::isAttached(const AppenderPtr appender) const
{
- log4cxx::shared_lock<log4cxx::shared_mutex> lock(mutex);
-
- if (appender == 0 || aai == 0)
- {
- return false;
- }
- else
- {
- return aai->isAttached(appender);
- }
+ return aai->isAttached(appender);
}
bool Logger::isTraceEnabled() const
@@ -471,36 +426,16 @@ void Logger::l7dlog(const LevelPtr& level1, const
std::string& key,
void Logger::removeAllAppenders()
{
- std::unique_lock<log4cxx::shared_mutex> lock(mutex);
-
- if (aai != 0)
- {
- aai->removeAllAppenders();
- aai = 0;
- }
+ aai->removeAllAppenders();
}
void Logger::removeAppender(const AppenderPtr appender)
{
- std::unique_lock<log4cxx::shared_mutex> lock(mutex);
-
- if (appender == 0 || aai == 0)
- {
- return;
- }
-
aai->removeAppender(appender);
}
void Logger::removeAppender(const LogString& name1)
{
- std::unique_lock<log4cxx::shared_mutex> lock(mutex);
-
- if (name1.empty() || aai == 0)
- {
- return;
- }
-
aai->removeAppender(name1);
}
diff --git a/src/main/cpp/rollingfileappender.cpp
b/src/main/cpp/rollingfileappender.cpp
index bb805a6..3e6f2dc 100644
--- a/src/main/cpp/rollingfileappender.cpp
+++ b/src/main/cpp/rollingfileappender.cpp
@@ -39,6 +39,7 @@
#include <log4cxx/helpers/bytebuffer.h>
#include <log4cxx/rolling/fixedwindowrollingpolicy.h>
#include <log4cxx/rolling/manualtriggeringpolicy.h>
+#include <log4cxx/helpers/transcoder.h>
using namespace log4cxx;
using namespace log4cxx::rolling;
@@ -305,9 +306,12 @@ bool RollingFileAppenderSkeleton::rolloverInternal(Pool& p)
{
success
= rollover1->getSynchronous()->execute(p);
}
- catch
(std::exception&)
+ catch
(std::exception& ex)
{
LogLog::warn(LOG4CXX_STR("Exception on rollover"));
+
LogString exmsg;
+
log4cxx::helpers::Transcoder::decode(ex.what(), exmsg);
+
errorHandler->error(exmsg, ex, 0);
}
}
@@ -363,9 +367,12 @@ bool RollingFileAppenderSkeleton::rolloverInternal(Pool& p)
{
success
= rollover1->getSynchronous()->execute(p);
}
- catch
(std::exception&)
+ catch
(std::exception& ex)
{
LogLog::warn(LOG4CXX_STR("Exception during rollover"));
+
LogString exmsg;
+
log4cxx::helpers::Transcoder::decode(ex.what(), exmsg);
+
errorHandler->error(exmsg, ex, 0);
}
}
@@ -400,9 +407,12 @@ bool RollingFileAppenderSkeleton::rolloverInternal(Pool& p)
return true;
}
}
- catch (std::exception&)
+ catch (std::exception& ex)
{
LogLog::warn(LOG4CXX_STR("Exception
during rollover"));
+ LogString exmsg;
+
log4cxx::helpers::Transcoder::decode(ex.what(), exmsg);
+ errorHandler->error(exmsg, ex, 0);
}
#ifdef LOG4CXX_MULTI_PROCESS
@@ -458,9 +468,12 @@ void RollingFileAppenderSkeleton::subAppend(const
LoggingEventPtr& event, Pool&
_event = &(const_cast<LoggingEventPtr&>(event));
rolloverInternal(p);
}
- catch (std::exception&)
+ catch (std::exception& ex)
{
LogLog::warn(LOG4CXX_STR("Exception during rollover
attempt."));
+ LogString exmsg;
+ log4cxx::helpers::Transcoder::decode(ex.what(), exmsg);
+ errorHandler->error(exmsg);
}
}
