Bug 46163 - LoggerDynamicMBean needs to handle a null Appender name
LoggerDynamicMBean needs to handle a null Appender name
Product: Log4j
Classification: Unclassified
Component: Other
PC Mac OS X 10.4
: P2 normal
: ---
Assigned To: log4j-dev
Depends on:
  Show dependency tree
Reported: 2008-11-06 16:17 UTC by Paul Smith
Modified: 2009-01-14 10:56 UTC (History)
1 user (show)

Patch that fixes the null appender name (5.59 KB, patch)
2009-01-13 13:11 UTC, Paul Smith
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Paul Smith 2008-11-06 16:17:06 UTC
If a plain BasicConfigurator.configure() is called in an application after the HierarchyMBean is instantiated, then an exception is thrown trying to create an ObjectName for the appender based on the Appender's name property, which is initialised to null, and is not allowed.

Instead, the LoggerDynamicMBean class should be defensive when creating a Appender MBean's by detecting a null/empty string name and at least trying to use .toString() to construct a logical display name.
Comment 1 Paul Smith 2008-11-09 15:16:06 UTC
I have a simple patch for this that I'm just testing, it basically resorts to defensively using the Appender.toString() in an attempt to get at least a value for the JMX namespace.  If that fails, well, then you're screwed still.
Comment 2 QualityChecker 2008-11-10 03:19:36 UTC
You'll find here http://d.cr.free.fr/wswebconsulterfichiers.php?projet=demojava_log4j&mode=res&fichier=org.apache.log4j.jmx.LoggerDynamicMBean.java.res&fichierxsl=
a collection of mistakes and errors. Most are easy to fix.

The entire log4j project is available here : http://d.cr.free.fr/wswebconsulterfichiers.php?projet=demojava_log4j

If of interest for you, thanks for your (positive..) feedback.
Coming soon : CWE identification of bugs. 
Comment 3 Curt Arnold 2009-01-13 12:30:28 UTC
Paul, do you want to submit your patch?
Comment 4 Paul Smith 2009-01-13 13:11:33 UTC
Created attachment 23112 [details]
Patch that fixes the null appender name

Note: This is a direct diff of my workspace, which also includes a change to test/build.xml to remove the ErrorHandler test that just doesn't pass on my computer for some unknown reason (before and after my change).

We've been using this change in production for 2 months now with no problems.
Comment 5 Curt Arnold 2009-01-14 10:56:17 UTC
Committed changes in rev 734480.

I did a few things different than Paul's patch.  First, Paul's patch used a package visible static method to handle the logic to get a name from the appender.  I moved the function into a common ancestor and made it protected.  Also, Paul's patch removed use of the layout from the AppenderDynamicMBean name.  That may be a desirable change, but it wasn't part of this bug report.