Fix bug #44627 - improve the thread safety of POILogFactory

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@638815 13f79535-47bb-0310-9956-ffa450edef68

src/documentation/content/xdocs/changes.xml

@@ -36,6 +36,7 @@
 
 		<!-- Don't forget to update status.xml too! -->
         <release version="3.1-beta1" date="2008-??-??">
+           <action dev="POI-DEVELOPERS" type="fix">44627 - Improve the thread safety of POILogFactory</action>
            <action dev="POI-DEVELOPERS" type="add">30311 - Initial support for Conditional Formatting</action>
            <action dev="POI-DEVELOPERS" type="fix">44609 - Handle leading spaces in formulas, such as '= 4'</action>
            <action dev="POI-DEVELOPERS" type="add">44608 - Support for PercentPtg in the formula evaluator</action>

src/documentation/content/xdocs/status.xml

@@ -33,6 +33,7 @@
 	<!-- Don't forget to update changes.xml too! -->
     <changes>
         <release version="3.1-beta1" date="2008-??-??">
+           <action dev="POI-DEVELOPERS" type="fix">44627 - Improve the thread safety of POILogFactory</action>
            <action dev="POI-DEVELOPERS" type="add">30311 - Initial support for Conditional Formatting</action>
            <action dev="POI-DEVELOPERS" type="fix">44609 - Handle leading spaces in formulas, such as '= 4'</action>
            <action dev="POI-DEVELOPERS" type="add">44608 - Support for PercentPtg in the formula evaluator</action>

src/java/org/apache/poi/util/POILogFactory.java

@@ -33,14 +33,25 @@ import java.util.*;
 public class POILogFactory
 {
 
-    // map of POILogger instances, with classes as keys
+    /**
+     * Map of POILogger instances, with classes as keys
+     */
     private static Map _loggers = new HashMap();;
 
+    /**
+     * A common instance of NullLogger, as it does nothing
+     *  we only need the one
+     */
+    private static POILogger _nullLogger = new NullLogger();
+    /**
+     * The name of the class to use. Initialised the
+     *  first time we need it
+     */
+    private static String _loggerClassName = null;
 
     /**
-     * construct a POILogFactory.
+     * Construct a POILogFactory.
      */
-
     private POILogFactory()
     {
     }
@@ -70,27 +81,47 @@ public class POILogFactory
     {
         POILogger logger = null;
         
-        if (_loggers.containsKey(cat))
+        // If we haven't found out what logger to use yet,
-        {
+        //  then do so now
-            logger = ( POILogger ) _loggers.get(cat);
+        // Don't look it up until we're first asked, so
-        }
+        //  that our users can set the system property
-        else
+        //  between class loading and first use
-        {
+        if(_loggerClassName == null) {
         	try {
-              String loggerClassName = System.getProperty("org.apache.poi.util.POILogger");
+        		_loggerClassName = System.getProperty("org.apache.poi.util.POILogger");
-              Class loggerClass = Class.forName(loggerClassName);
+        	} catch(Exception e) {}
+        	
+        	// Use the default logger if none specified,
+        	//  or none could be fetched
+        	if(_loggerClassName == null) {
+        		_loggerClassName = _nullLogger.getClass().getName();
+        	}
+        }
+        
+        // Short circuit for the null logger, which
+        //  ignores all categories
+        if(_loggerClassName.equals(_nullLogger.getClass().getName())) {
+        	return _nullLogger;
+        }
+
+        
+        // Fetch the right logger for them, creating
+        //  it if that's required 
+        if (_loggers.containsKey(cat)) {
+            logger = ( POILogger ) _loggers.get(cat);
+        } else {
+            try {
+              Class loggerClass = Class.forName(_loggerClassName);
               logger = ( POILogger ) loggerClass.newInstance();
-            }
-            catch(Exception e){
-            
-              logger = new NullLogger();
-            }
-            
               logger.initialize(cat);
+            } catch(Exception e) {
+              // Give up and use the null logger
+              logger = _nullLogger;
+            }
             
+            // Save for next time
             _loggers.put(cat, logger);
         }
         return logger;
     }
-        
 }   // end public class POILogFactory

src/testcases/org/apache/poi/poifs/storage/TestRawDataBlock.java

@@ -36,17 +36,7 @@ import junit.framework.*;
 public class TestRawDataBlock
     extends TestCase
 {
-
+	static {
-    /**
-     * Constructor TestRawDataBlock
-     *
-     * @param name
-     */
-
-    public TestRawDataBlock(String name)
-    {
-        super(name);
-        
         // We always want to use our own
         //  logger
         System.setProperty(
@@ -55,6 +45,16 @@ public class TestRawDataBlock
         );
 	}
 
+    /**
+     * Constructor TestRawDataBlock
+     *
+     * @param name
+     */
+    public TestRawDataBlock(String name)
+    {
+        super(name);
+    }
+
     /**
      * Test creating a normal RawDataBlock
      *

src/testcases/org/apache/poi/poifs/storage/TestRawDataBlockList.java

@@ -36,17 +36,7 @@ import junit.framework.*;
 public class TestRawDataBlockList
     extends TestCase
 {
-
+	static {
-    /**
-     * Constructor TestRawDataBlockList
-     *
-     * @param name
-     */
-
-    public TestRawDataBlockList(String name)
-    {
-        super(name);
-        
         // We always want to use our own
         //  logger
         System.setProperty(
@@ -55,12 +45,21 @@ public class TestRawDataBlockList
         );
 	}
 
+    /**
+     * Constructor TestRawDataBlockList
+     *
+     * @param name
+     */
+    public TestRawDataBlockList(String name)
+    {
+        super(name);
+    }
+
     /**
      * Test creating a normal RawDataBlockList
      *
      * @exception IOException
      */
-
     public void testNormalConstructor()
         throws IOException
     {