svn commit: r169387 - /jakarta/commons/proper/logging/trunk/src/java/org/apache/commons/logging/LogFactory.java

classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

svn commit: r169387 - /jakarta/commons/proper/logging/trunk/src/java/org/apache/commons/logging/LogFactory.java

Simon Kitching
Author: skitching
Date: Mon May  9 17:45:18 2005
New Revision: 169387

URL: http://svn.apache.org/viewcvs?rev=169387&view=rev
Log:
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.

Modified:
    jakarta/commons/proper/logging/trunk/src/java/org/apache/commons/logging/LogFactory.java

Modified: jakarta/commons/proper/logging/trunk/src/java/org/apache/commons/logging/LogFactory.java
URL: http://svn.apache.org/viewcvs/jakarta/commons/proper/logging/trunk/src/java/org/apache/commons/logging/LogFactory.java?rev=169387&r1=169386&r2=169387&view=diff
==============================================================================
--- jakarta/commons/proper/logging/trunk/src/java/org/apache/commons/logging/LogFactory.java (original)
+++ jakarta/commons/proper/logging/trunk/src/java/org/apache/commons/logging/LogFactory.java Mon May  9 17:45:18 2005
@@ -117,7 +117,7 @@
         "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 @@
      */
     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 @@
         if (factory != null)
             return factory;
 
-
         // Load properties file.
         // Will be used one way or another in the end.
 
@@ -445,10 +460,17 @@
     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 @@
                 element.release();
             }
             factories.clear();
+
+            if (nullClassLoaderFactory != null) {
+                nullClassLoaderFactory.release();
+                nullClassLoaderFactory = null;
+            }
         }
 
     }
@@ -542,21 +569,47 @@
 
     /**
      * 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);
+            }
+        }
     }
 
     /**



---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]