You are viewing a plain text version of this content. The canonical link for it is here.
Posted to log4cxx-dev@logging.apache.org by Jens Hannemann <j....@ieee.org> on 2006/09/22 18:31:49 UTC
Handling failure in log4cxx::PropertyConfigurator::configure()
Hi folks,
I recently started using log4cxx (from the svn trunk). Thanks for the great
work. It is easily the most featureful logging framework out there, althout
the Java legacy makes it somewhat awkward to use in the C++ world.
I wanted to be able to run my executables with different log configuration
files for different tasks such as performance evaluation or debugging, and
thus wanted to be able to pass the configuration file name to use as a
command line option. The problem is that there is no mechanism in the
log4cxx::PropertyConfigurator::configure() method that allows me to check for
failure of the configuration. If the file cannot be read or is malformed,
I'll get an error message to the console but nothing else. So I checked the
source code and saw that the doConfigure method actually uses exceptions
(albeit is not exception-safe) but chooses to not let the exception propagate
up the call stack to let the user handle it (which is one of the main reasons
for using exceptions in the first place). So I decided to remove the
try-catch blocks from the code so I could handle the exceptions myself like
this:
try{
// try to configure the logger with the given filename
// if the file could not be processed, we'll get an exception
// CAUTION: This relies on a patch to the
// propertyconfigurator.cpp file of log4cxx
log4cxx::PropertyConfigurator::configure(log_cfg_filename);
}
catch (std::exception &e){
// do nothing, stay with default
}
That didn't work as I thought as the code is not exception safe. Before it
even enters its own try block, it sets the hierarchy configured state to
true:
hierarchy->setConfigured(true);
When an exception is thrown, this either needs to be reverted in the catch
blocks, or - easier yet - set the configured state *after* the configuration
was successful.
The following patch does just that: lets the exceptions propagate up the call
stack and makes the method exception-safe by setting the hierarchy only after
success:
=== begin patch ===
Index: src/propertyconfigurator.cpp
===================================================================
--- src/propertyconfigurator.cpp (revision 448948)
+++ src/propertyconfigurator.cpp (working copy)
@@ -81,24 +81,11 @@
void PropertyConfigurator::doConfigure(const File& configFileName,
spi::LoggerRepositoryPtr& hierarchy)
{
- hierarchy->setConfigured(true);
-
Properties props;
- try {
- InputStreamPtr inputStream = new FileInputStream(configFileName);
- props.load(inputStream);
- } catch(const IOException& ie) {
- LogLog::error(((LogString) LOG4CXX_STR("Could not read
configuration file ["))
- + configFileName.getName() + LOG4CXX_STR("]."));
- return;
- }
-
- try {
- doConfigure(props, hierarchy);
- } catch(const std::exception& ex) {
- LogLog::error(((LogString) LOG4CXX_STR("Could not parse
configuration file ["))
- + configFileName.getName() + LOG4CXX_STR("]."), ex);
- }
+ InputStreamPtr inputStream = new FileInputStream(configFileName);
+ props.load(inputStream);
+ doConfigure(props, hierarchy);
+ hierarchy->setConfigured(true);
}
void PropertyConfigurator::configure(const File& configFilename)
=== end patch ===
There's also a bug in Logger::getLogger(): it trashes the system (eating up
memory until it dies) when being accidentally passed a name with a leading
"." (dot). I'll try to file that as a bug.
Best regards,
Jens
--
Dr.-Ing. Jens Hannemann --- j.hannemann@ieee.org --- GPG Key Available
University of Kentucky -- Dept. of Electrical and Computer Engineering
Center for Visualization and Virtual Environments
Re: Handling failure in log4cxx::PropertyConfigurator::configure()
Posted by Curt Arnold <ca...@apache.org>.
I agree that silently consuming exceptions in the configurator is
undesirable. log4cxx mimic log4j at this point and it is not
possible to change the existing log4j contract at this point, but
log4cxx is not necessarily bound to mimic it.
FYI: I started a sandbox effort for a XML configuration syntax
(strictxml) that can be effectively validated by an XML schema
validator and the corresponding log4j configurator is designed to
allow the caller to either catch exceptions or to add a error handler
to continue through errors. That configurator also attempts to defer
any configuration change until the entire document has been parsed
and processed, so you do not get a partially configured system. That
effort however is only partially implemented and no attempt has been
made to start a port to log4cxx and log4net.
Thanks for the other report in getLogger("."). Please file a bug
when you get a chance.
On Sep 22, 2006, at 11:31 AM, Jens Hannemann wrote:
> Hi folks,
>
> I recently started using log4cxx (from the svn trunk). Thanks for
> the great
> work. It is easily the most featureful logging framework out there,
> althout
> the Java legacy makes it somewhat awkward to use in the C++ world.
>
> I wanted to be able to run my executables with different log
> configuration
> files for different tasks such as performance evaluation or
> debugging, and
> thus wanted to be able to pass the configuration file name to use as a
> command line option. The problem is that there is no mechanism in the
> log4cxx::PropertyConfigurator::configure() method that allows me to
> check for
> failure of the configuration. If the file cannot be read or is
> malformed,
> I'll get an error message to the console but nothing else. So I
> checked the
> source code and saw that the doConfigure method actually uses
> exceptions
> (albeit is not exception-safe) but chooses to not let the exception
> propagate
> up the call stack to let the user handle it (which is one of the
> main reasons
> for using exceptions in the first place). So I decided to remove the
> try-catch blocks from the code so I could handle the exceptions
> myself like
> this:
>
> try{
> // try to configure the logger with the given filename
> // if the file could not be processed, we'll get an exception
> // CAUTION: This relies on a patch to the
> // propertyconfigurator.cpp file of log4cxx
> log4cxx::PropertyConfigurator::configure(log_cfg_filename);
> }
> catch (std::exception &e){
> // do nothing, stay with default
> }
>
> That didn't work as I thought as the code is not exception safe.
> Before it
> even enters its own try block, it sets the hierarchy configured
> state to
> true:
>
> hierarchy->setConfigured(true);
>
> When an exception is thrown, this either needs to be reverted in
> the catch
> blocks, or - easier yet - set the configured state *after* the
> configuration
> was successful.
>
> The following patch does just that: lets the exceptions propagate
> up the call
> stack and makes the method exception-safe by setting the hierarchy
> only after
> success:
>
> === begin patch ===
>
> Index: src/propertyconfigurator.cpp
> ===================================================================
> --- src/propertyconfigurator.cpp (revision 448948)
> +++ src/propertyconfigurator.cpp (working copy)
> @@ -81,24 +81,11 @@
> void PropertyConfigurator::doConfigure(const File& configFileName,
> spi::LoggerRepositoryPtr& hierarchy)
> {
> - hierarchy->setConfigured(true);
> -
> Properties props;
> - try {
> - InputStreamPtr inputStream = new FileInputStream
> (configFileName);
> - props.load(inputStream);
> - } catch(const IOException& ie) {
> - LogLog::error(((LogString) LOG4CXX_STR("Could not read
> configuration file ["))
> - + configFileName.getName() + LOG4CXX_STR
> ("]."));
> - return;
> - }
> -
> - try {
> - doConfigure(props, hierarchy);
> - } catch(const std::exception& ex) {
> - LogLog::error(((LogString) LOG4CXX_STR("Could not parse
> configuration file ["))
> - + configFileName.getName() + LOG4CXX_STR
> ("]."), ex);
> - }
> + InputStreamPtr inputStream = new FileInputStream
> (configFileName);
> + props.load(inputStream);
> + doConfigure(props, hierarchy);
> + hierarchy->setConfigured(true);
> }
>
> void PropertyConfigurator::configure(const File& configFilename)
>
> === end patch ===
>
> There's also a bug in Logger::getLogger(): it trashes the system
> (eating up
> memory until it dies) when being accidentally passed a name with a
> leading
> "." (dot). I'll try to file that as a bug.
>
> Best regards,
>
> Jens
>
> --
> Dr.-Ing. Jens Hannemann --- j.hannemann@ieee.org --- GPG Key Available
> University of Kentucky -- Dept. of Electrical and Computer Engineering
> Center for Visualization and Virtual Environments