Skip to content

Commit

Permalink
Fix HierarchyEventListener (#295)
Browse files Browse the repository at this point in the history 
Fix HierarchyEventListener

See #286
  • Loading branch information
@rm5248
rm5248 authored Nov 20, 2023
1 parent 1fd84a4 commit 2a381ae
Show file tree
Hide file tree
Showing 3 changed files with 117 additions and 16 deletions.
30 changes: 15 additions & 15 deletions src/main/cpp/hierarchy.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ struct Hierarchy::HierarchyPrivate
}

helpers::Pool pool;
mutable std::mutex mutex;
mutable std::recursive_mutex mutex;
mutable std::mutex configuredMutex;
bool configured;
bool emittedNoAppenderWarning;
Expand All @@ -81,7 +81,7 @@ Hierarchy::Hierarchy() :

Hierarchy::~Hierarchy()
{
std::unique_lock<std::mutex> lock(m_priv->mutex);
std::lock_guard<std::recursive_mutex> lock(m_priv->mutex);
for (auto& item : m_priv->loggers)
{
if (auto& pLogger = item.second)
Expand All @@ -99,7 +99,7 @@ Hierarchy::~Hierarchy()

void Hierarchy::addHierarchyEventListener(const spi::HierarchyEventListenerPtr& listener)
{
std::unique_lock<std::mutex> lock(m_priv->mutex);
std::lock_guard<std::recursive_mutex> lock(m_priv->mutex);

if (std::find(m_priv->listeners.begin(), m_priv->listeners.end(), listener) != m_priv->listeners.end())
{
Expand All @@ -113,7 +113,7 @@ void Hierarchy::addHierarchyEventListener(const spi::HierarchyEventListenerPtr&

void Hierarchy::removeHierarchyEventListener(const spi::HierarchyEventListenerPtr& listener)
{
std::unique_lock<std::mutex> lock(m_priv->mutex);
std::lock_guard<std::recursive_mutex> lock(m_priv->mutex);

auto found = std::find(m_priv->listeners.begin(), m_priv->listeners.end(), listener);
if(found != m_priv->listeners.end()){
Expand All @@ -123,15 +123,15 @@ void Hierarchy::removeHierarchyEventListener(const spi::HierarchyEventListenerPt

void Hierarchy::clear()
{
std::unique_lock<std::mutex> lock(m_priv->mutex);
std::lock_guard<std::recursive_mutex> lock(m_priv->mutex);
m_priv->loggers.clear();
}

void Hierarchy::emitNoAppenderWarning(const Logger* logger)
{
bool emitWarning = false;
{
std::unique_lock<std::mutex> lock(m_priv->mutex);
std::lock_guard<std::recursive_mutex> lock(m_priv->mutex);
emitWarning = !m_priv->emittedNoAppenderWarning;
m_priv->emittedNoAppenderWarning = true;
}
Expand All @@ -148,7 +148,7 @@ void Hierarchy::emitNoAppenderWarning(const Logger* logger)

LoggerPtr Hierarchy::exists(const LogString& name)
{
std::unique_lock<std::mutex> lock(m_priv->mutex);
std::lock_guard<std::recursive_mutex> lock(m_priv->mutex);

LoggerPtr logger;
LoggerMap::iterator it = m_priv->loggers.find(name);
Expand All @@ -166,7 +166,7 @@ void Hierarchy::setThreshold(const LevelPtr& l)
{
if (l != 0)
{
std::unique_lock<std::mutex> lock(m_priv->mutex);
std::lock_guard<std::recursive_mutex> lock(m_priv->mutex);
setThresholdInternal(l);
}
}
Expand Down Expand Up @@ -202,7 +202,7 @@ void Hierarchy::fireAddAppenderEvent(const Logger* logger, const Appender* appen
setConfigured(true);
HierarchyEventListenerList clonedList;
{
std::unique_lock<std::mutex> lock(m_priv->mutex);
std::lock_guard<std::recursive_mutex> lock(m_priv->mutex);
clonedList = m_priv->listeners;
}

Expand All @@ -221,7 +221,7 @@ void Hierarchy::fireRemoveAppenderEvent(const Logger* logger, const Appender* ap
{
HierarchyEventListenerList clonedList;
{
std::unique_lock<std::mutex> lock(m_priv->mutex);
std::lock_guard<std::recursive_mutex> lock(m_priv->mutex);
clonedList = m_priv->listeners;
}
HierarchyEventListenerList::iterator it, itEnd = clonedList.end();
Expand Down Expand Up @@ -249,7 +249,7 @@ LoggerPtr Hierarchy::getLogger(const LogString& name,
const spi::LoggerFactoryPtr& factory)
{
auto root = getRootLogger();
std::unique_lock<std::mutex> lock(m_priv->mutex);
std::lock_guard<std::recursive_mutex> lock(m_priv->mutex);

LoggerMap::iterator it = m_priv->loggers.find(name);
LoggerPtr result;
Expand Down Expand Up @@ -281,7 +281,7 @@ LoggerPtr Hierarchy::getLogger(const LogString& name,

LoggerList Hierarchy::getCurrentLoggers() const
{
std::unique_lock<std::mutex> lock(m_priv->mutex);
std::lock_guard<std::recursive_mutex> lock(m_priv->mutex);

LoggerList v;
for (auto& item : m_priv->loggers)
Expand All @@ -294,7 +294,7 @@ LoggerList Hierarchy::getCurrentLoggers() const

LoggerPtr Hierarchy::getRootLogger() const
{
std::unique_lock<std::mutex> lock(m_priv->mutex);
std::lock_guard<std::recursive_mutex> lock(m_priv->mutex);
if (!m_priv->root)
{
m_priv->root = std::make_shared<RootLogger>(m_priv->pool, Level::getDebug());
Expand All @@ -321,7 +321,7 @@ void Hierarchy::ensureIsConfigured(std::function<void()> configurator)

void Hierarchy::resetConfiguration()
{
std::unique_lock<std::mutex> lock(m_priv->mutex);
std::lock_guard<std::recursive_mutex> lock(m_priv->mutex);

if (m_priv->root)
{
Expand Down Expand Up @@ -349,7 +349,7 @@ void Hierarchy::resetConfiguration()

void Hierarchy::shutdown()
{
std::unique_lock<std::mutex> lock(m_priv->mutex);
std::lock_guard<std::recursive_mutex> lock(m_priv->mutex);

shutdownInternal();
}
Expand Down
17 changes: 16 additions & 1 deletion src/main/cpp/logger.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -502,17 +502,32 @@ void Logger::l7dlog(const LevelPtr& level1, const std::string& key,

void Logger::removeAllAppenders()
{
AppenderList currentAppenders = m_priv->aai.getAllAppenders();
m_priv->aai.removeAllAppenders();

auto rep = getHierarchy();
if(rep){
for(AppenderPtr appender : currentAppenders){
rep->fireRemoveAppenderEvent(this, appender.get());
}
}
}

void Logger::removeAppender(const AppenderPtr appender)
{
m_priv->aai.removeAppender(appender);
if (auto rep = getHierarchy())
{
rep->fireRemoveAppenderEvent(this, appender.get());
}
}

void Logger::removeAppender(const LogString& name1)
{
m_priv->aai.removeAppender(name1);
AppenderPtr appender = m_priv->aai.getAppender(name1);
if(appender){
removeAppender(appender);
}
}

void Logger::removeHierarchy()
Expand Down
86 changes: 86 additions & 0 deletions src/test/cpp/loggertestcase.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,31 @@ class CountingAppender : public AppenderSkeleton
}
};

class CountingListener : public HierarchyEventListener{
public:
DECLARE_LOG4CXX_OBJECT(CountingListener)
BEGIN_LOG4CXX_CAST_MAP()
LOG4CXX_CAST_ENTRY(CountingListener)
END_LOG4CXX_CAST_MAP()

int numRemoved = 0;
int numAdded = 0;

void addAppenderEvent(
const Logger* logger,
const Appender* appender){
numAdded++;
}

void removeAppenderEvent(
const Logger* logger,
const Appender* appender){
numRemoved++;
}
};

IMPLEMENT_LOG4CXX_OBJECT(CountingListener);

LOGUNIT_CLASS(LoggerTestCase)
{
LOGUNIT_TEST_SUITE(LoggerTestCase);
Expand All @@ -76,6 +101,9 @@ LOGUNIT_CLASS(LoggerTestCase)
LOGUNIT_TEST(testHierarchy1);
LOGUNIT_TEST(testTrace);
LOGUNIT_TEST(testIsTraceEnabled);
LOGUNIT_TEST(testAddingListeners);
LOGUNIT_TEST(testAddingAndRemovingListeners);
LOGUNIT_TEST(testAddingAndRemovingListeners2);
LOGUNIT_TEST_SUITE_END();

public:
Expand Down Expand Up @@ -488,6 +516,64 @@ LOGUNIT_CLASS(LoggerTestCase)
LOGUNIT_ASSERT_EQUAL(true, Logger::isErrorEnabledFor(root));
}

void testAddingListeners()
{
auto appender = std::shared_ptr<CountingAppender>(new CountingAppender);
LoggerPtr root = Logger::getRootLogger();
std::shared_ptr<CountingListener> listener = std::shared_ptr<CountingListener>(new CountingListener());

root->getLoggerRepository()->addHierarchyEventListener(listener);

root->addAppender(appender);

LOGUNIT_ASSERT_EQUAL(1, listener->numAdded);
LOGUNIT_ASSERT_EQUAL(0, listener->numRemoved);
}

void testAddingAndRemovingListeners()
{
auto appender = std::shared_ptr<CountingAppender>(new CountingAppender);
LoggerPtr root = Logger::getRootLogger();
std::shared_ptr<CountingListener> listener = std::shared_ptr<CountingListener>(new CountingListener());

root->getLoggerRepository()->addHierarchyEventListener(listener);

root->addAppender(appender);

LOGUNIT_ASSERT_EQUAL(1, listener->numAdded);
LOGUNIT_ASSERT_EQUAL(0, listener->numRemoved);

root->removeAppender(appender);

LOGUNIT_ASSERT_EQUAL(1, listener->numAdded);
LOGUNIT_ASSERT_EQUAL(1, listener->numRemoved);
}

void testAddingAndRemovingListeners2()
{
auto appender = std::shared_ptr<CountingAppender>(new CountingAppender);
auto appender2 = std::shared_ptr<CountingAppender>(new CountingAppender);
LoggerPtr root = Logger::getRootLogger();
std::shared_ptr<CountingListener> listener = std::shared_ptr<CountingListener>(new CountingListener());

root->getLoggerRepository()->addHierarchyEventListener(listener);

root->addAppender(appender);

LOGUNIT_ASSERT_EQUAL(1, listener->numAdded);
LOGUNIT_ASSERT_EQUAL(0, listener->numRemoved);

root->addAppender(appender2);

LOGUNIT_ASSERT_EQUAL(2, listener->numAdded);
LOGUNIT_ASSERT_EQUAL(0, listener->numRemoved);

root->removeAllAppenders();

LOGUNIT_ASSERT_EQUAL(2, listener->numAdded);
LOGUNIT_ASSERT_EQUAL(2, listener->numRemoved);
}

protected:
static LogString MSG;
LoggerPtr logger;
Expand Down

0 comments on commit 2a381ae

Please sign in to comment.