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
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 @@
return;
}
+ std::unique_lock<std::mutex> lock( m_mutex );
AppenderList::iterator it = std::find(
appenderList.begin(), appenderList.end(), newAppender);
@@ -55,14 +56,27 @@
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 @@
return 0;
}
+ std::unique_lock<std::mutex> lock( m_mutex );
AppenderList::const_iterator it, itEnd = appenderList.end();
AppenderPtr appender;
@@ -100,6 +115,7 @@
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 @@
void AppenderAttachableImpl::removeAllAppenders()
{
+ std::unique_lock<std::mutex> lock( m_mutex );
AppenderList::iterator it, itEnd = appenderList.end();
AppenderPtr a;
@@ -127,6 +144,7 @@
return;
}
+ std::unique_lock<std::mutex> lock( m_mutex );
AppenderList::iterator it = std::find(
appenderList.begin(), appenderList.end(), appender);
@@ -143,6 +161,7 @@
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 @@
void AsyncAppender::addAppender(const AppenderPtr newAppender)
{
- std::unique_lock<std::mutex> lock(appenders->getMutex());
appenders->addAppender(newAppender);
}
@@ -221,19 +220,16 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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::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 @@
{
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 @@
{
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 @@
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 @@
_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);
}
}
