From 7839295a8ea19bee13369e2df17acdcf9e531ebf Mon Sep 17 00:00:00 2001
From: Simon Kitching <skitching@apache.org>
Date: Tue, 10 May 2005 00:45:18 +0000
Subject: [PATCH] Fix for case where classloader key to "factories" member is
 null. This can happen in JDK1.1 and in embedded systems work. Without this
 fix, a new LogFactoryImpl is created each time LogFactory.getLog(..) is
 called! See bugzilla#10825, comment#22. Thanks to Erik Erskine for bug report
 and fix.

git-svn-id: https://svn.apache.org/repos/asf/jakarta/commons/proper/logging/trunk@169387 13f79535-47bb-0310-9956-ffa450edef68
---
 .../apache/commons/logging/LogFactory.java    | 71 ++++++++++++++++---
 1 file changed, 62 insertions(+), 9 deletions(-)

diff --git a/src/java/org/apache/commons/logging/LogFactory.java b/src/java/org/apache/commons/logging/LogFactory.java
index 500fbe6..10339d2 100644
--- a/src/java/org/apache/commons/logging/LogFactory.java
+++ b/src/java/org/apache/commons/logging/LogFactory.java
@@ -117,7 +117,7 @@ public abstract class LogFactory {
         "org.apache.commons.logging.LogFactory.HashtableImpl";
     /** Name used to load the weak hashtable implementation by names */
     private static final String WEAK_HASHTABLE_CLASSNAME = "org.apache.commons.logging.impl.WeakHashtable";
-    
+
     // ----------------------------------------------------------- Constructors
 
 
@@ -221,6 +221,22 @@ public abstract class LogFactory {
      */
     protected static Hashtable factories = createFactoryStore();
     
+    /**
+     * Prevously constructed <code>LogFactory</code> instance as in the
+     * <code>factories</code> map. but for the case where
+     * <code>getClassLoader</code> returns <code>null</code>.
+     * This can happen when:
+     * <ul>
+     * <li>using JDK1.1 and the calling code is loaded via the system
+     *  classloader (very common)</li>
+     * <li>using JDK1.2+ and the calling code is loaded via the boot
+     *  classloader (only likely for embedded systems work).</li>
+     * </ul>
+     * Note that <code>factories</code> is a <i>Hashtable</i> (not a HashMap),
+     * and hashtables don't allow null as a key.
+     */
+    protected static LogFactory nullClassLoaderFactory = null;
+
     private static final Hashtable createFactoryStore() {
         Hashtable result = null;
         String storeImplementationClass 
@@ -290,7 +306,6 @@ public abstract class LogFactory {
         if (factory != null)
             return factory;
 
-
         // Load properties file.
         // Will be used one way or another in the end.
 
@@ -445,10 +460,17 @@ public abstract class LogFactory {
     public static void release(ClassLoader classLoader) {
 
         synchronized (factories) {
-            LogFactory factory = (LogFactory) factories.get(classLoader);
-            if (factory != null) {
-                factory.release();
-                factories.remove(classLoader);
+            if (classLoader == null) {
+                if (nullClassLoaderFactory != null) {
+                    nullClassLoaderFactory.release();
+                    nullClassLoaderFactory = null;
+                }
+            } else {
+                LogFactory factory = (LogFactory) factories.get(classLoader);
+                if (factory != null) {
+                    factory.release();
+                    factories.remove(classLoader);
+                }
             }
         }
 
@@ -472,6 +494,11 @@ public abstract class LogFactory {
                 element.release();
             }
             factories.clear();
+
+            if (nullClassLoaderFactory != null) {
+                nullClassLoaderFactory.release();
+                nullClassLoaderFactory = null;
+            }
         }
 
     }
@@ -542,21 +569,47 @@ public abstract class LogFactory {
 
     /**
      * Check cached factories (keyed by contextClassLoader)
+     *
+     * @return the factory associated with the specified classloader if
+     * one has previously been created, or null if this is the first time
+     * we have seen this particular classloader.
      */
     private static LogFactory getCachedFactory(ClassLoader contextClassLoader)
     {
         LogFactory factory = null;
 
-        if (contextClassLoader != null)
+        if (contextClassLoader == null) {
+            // nb: nullClassLoaderFactory might be null. That's ok.
+            factory = nullClassLoaderFactory;
+        } else {
             factory = (LogFactory) factories.get(contextClassLoader);
+        }
 
         return factory;
     }
 
+    /**
+     * Remember this factory, so later calls to LogFactory.getCachedFactory
+     * can return the previously created object (together with all its
+     * cached Log objects).
+     *
+     * @param classLoader should be the current context classloader. Note that
+     * this can be null under some circumstances; this is ok.
+     *
+     * @param factory should be the factory to cache. This should never be null.
+     */
     private static void cacheFactory(ClassLoader classLoader, LogFactory factory)
     {
-        if (classLoader != null && factory != null)
-            factories.put(classLoader, factory);
+        // Ideally we would assert(factory != null) here. However reporting
+        // errors from within a logging implementation is a little tricky!
+
+        if (factory != null) {
+            if (classLoader == null) {
+                nullClassLoaderFactory = factory;
+            } else {
+                factories.put(classLoader, factory);
+            }
+        }
     }
 
     /**