You are viewing a plain text version of this content. The canonical link for it is here.
Posted to log4cxx-user@logging.apache.org by Mihai Rusu <di...@roedu.net> on 2005/10/21 18:13:20 UTC
[PATCH][RFC] make Logger cache a LoggerRepositoryPtr instead of a "blind" pointer
Hi
The following patch (the biggest from the series) changes the codes so
that Logger caches a LoggerRepositoryPtr instead of a LoggerRepository*
otherwise the cached pointer can get destructed before it's last use (as
proved by valgrind and SIGSEGV). This patch fixes the first crash I got
with log4cxx when trying to log from a destructor of a static object.
Index: include/log4cxx/spi/loggerrepository.h
===================================================================
--- include/log4cxx/spi/loggerrepository.h (revision 688)
+++ include/log4cxx/spi/loggerrepository.h (revision 690)
@@ -41,6 +41,8 @@
class HierarchyEventListener;
typedef log4cxx::helpers::ObjectPtrT<HierarchyEventListener>
HierarchyEventListenerPtr;
+ class LoggerRepository;
+ typedef log4cxx::helpers::ObjectPtrT<LoggerRepository> LoggerRepositoryPtr;
/**
@@ -91,10 +93,12 @@
for an explanation. */
virtual const LevelPtr& getThreshold() const = 0;
- virtual LoggerPtr getLogger(const LogString& name) = 0;
+ virtual LoggerPtr getLogger(const LogString& name,
+ const spi::LoggerRepositoryPtr& lrepos) = 0;
virtual LoggerPtr getLogger(const LogString& name,
- const spi::LoggerFactoryPtr& factory) = 0;
+ const spi::LoggerFactoryPtr& factory,
+ const spi::LoggerRepositoryPtr& lrepos) = 0;
virtual LoggerPtr getRootLogger() const = 0;
Index: include/log4cxx/logger.h
===================================================================
--- include/log4cxx/logger.h (revision 688)
+++ include/log4cxx/logger.h (revision 690)
@@ -80,7 +80,7 @@
// Loggers need to know what Hierarchy they are in
- spi::LoggerRepository * repository;
+ spi::LoggerRepositoryPtr repository;
helpers::AppenderAttachableImplPtr aai;
@@ -537,7 +537,7 @@
friend class Hierarchy;
/**
Only the Hierarchy class can set the hierarchy of a logger.*/
- void setHierarchy(spi::LoggerRepository * repository);
+ void setHierarchy(const spi::LoggerRepositoryPtr& repository);
public:
/**
Index: include/log4cxx/hierarchy.h
===================================================================
--- include/log4cxx/hierarchy.h (revision 688)
+++ include/log4cxx/hierarchy.h (revision 690)
@@ -157,7 +157,8 @@
@param name The name of the logger to retrieve.
*/
- LoggerPtr getLogger(const LogString& name);
+ LoggerPtr getLogger(const LogString& name,
+ const spi::LoggerRepositoryPtr& lrepos);
/**
Return a new logger instance named as the first parameter using
@@ -173,7 +174,8 @@
*/
LoggerPtr getLogger(const LogString& name,
- const spi::LoggerFactoryPtr& factory);
+ const spi::LoggerFactoryPtr& factory,
+ const spi::LoggerRepositoryPtr& lrepos);
/**
Returns all the currently defined loggers in this hierarchy as
Index: src/hierarchy.cpp
===================================================================
--- src/hierarchy.cpp (revision 688)
+++ src/hierarchy.cpp (revision 690)
@@ -152,13 +152,15 @@
return threshold;
}
-LoggerPtr Hierarchy::getLogger(const LogString& name)
+LoggerPtr Hierarchy::getLogger(const LogString& name,
+ const spi::LoggerRepositoryPtr& lrepos)
{
- return getLogger(name, defaultFactory);
+ return getLogger(name, defaultFactory, lrepos);
}
LoggerPtr Hierarchy::getLogger(const LogString& name,
- const spi::LoggerFactoryPtr& factory)
+ const spi::LoggerFactoryPtr& factory,
+ const spi::LoggerRepositoryPtr& lrepos)
{
// Synchronize to prevent write conflicts. Read conflicts (in
// getEffectiveLevel method) are possible only if variable
@@ -177,7 +179,7 @@
{
logger = factory->makeNewLoggerInstance(name);
- logger->setHierarchy(this);
+ logger->setHierarchy(lrepos);
loggers.insert(LoggerMap::value_type(name, logger));
ProvisionNodeMap::iterator it2 = provisionNodes.find(name);
Index: src/logmanager.cpp
===================================================================
--- src/logmanager.cpp (revision 688)
+++ src/logmanager.cpp (revision 690)
@@ -96,7 +96,7 @@
*/
LoggerPtr LogManager::getLogger(const LogString& name)
{
- return getLoggerRepository()->getLogger(name);
+ return getLoggerRepository()->getLogger(name, getLoggerRepository());
}
/**
@@ -106,7 +106,7 @@
const spi::LoggerFactoryPtr& factory)
{
// Delegate the actual manufacturing of the logger to the logger repository.
- return getLoggerRepository()->getLogger(name, factory);
+ return getLoggerRepository()->getLogger(name, factory, getLoggerRepository());
}
LoggerPtr LogManager::exists(const std::string& name)
Index: src/logger.cpp
===================================================================
--- src/logger.cpp (revision 688)
+++ src/logger.cpp (revision 690)
@@ -517,7 +517,7 @@
this->additive = additive;
}
-void Logger::setHierarchy(spi::LoggerRepository * repository)
+void Logger::setHierarchy(const spi::LoggerRepositoryPtr& repository)
{
this->repository = repository;
}
Index: src/socketnode.cpp
===================================================================
--- src/socketnode.cpp (revision 691)
+++ src/socketnode.cpp (revision 692)
@@ -59,7 +59,7 @@
else
{
remoteLogger =
- hierarchy->getLogger(event->getLoggerName());
+ hierarchy->getLogger(event->getLoggerName(), hierarchy);
}
// apply the logger-level filter
Index: tests/src/loggertestcase.cpp
===================================================================
--- tests/src/loggertestcase.cpp (revision 691)
+++ tests/src/loggertestcase.cpp (revision 692)
@@ -387,12 +387,12 @@
{
LoggerPtr root(new RootCategory(Level::ERROR));
LoggerRepositoryPtr h = new Hierarchy(root);
- LoggerPtr a0 = h->getLogger(LOG4CXX_STR("a"));
+ LoggerPtr a0 = h->getLogger(LOG4CXX_STR("a"), h);
CPPUNIT_ASSERT_EQUAL((LogString) LOG4CXX_STR("a"), a0->getName());
CPPUNIT_ASSERT(a0->getLevel() == 0);
CPPUNIT_ASSERT(Level::ERROR == a0->getEffectiveLevel());
- LoggerPtr a1 = h->getLogger(LOG4CXX_STR("a"));
+ LoggerPtr a1 = h->getLogger(LOG4CXX_STR("a"), h);
CPPUNIT_ASSERT_EQUAL(a0, a1);
}
--
Mihai RUSU Email: dizzy@roedu.net
GPG : http://dizzy.roedu.net/dizzy-gpg.txt WWW: http://dizzy.roedu.net
"Linux is obsolete" -- AST