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

diff --git a/src/documentation/content/xdocs/changes.xml b/src/documentation/content/xdocs/changes.xml
index c2f3d50e13324f26f3c4401950e83bae737498d2..0f5b74ef25931554772cbc5311b6b5ac69f81614 100644 (file)
--- a/src/documentation/content/xdocs/changes.xml
+++ b/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>

diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml
index e0468b2f8792a7ec4f5a54a4d9879a6ab6899e52..5884c0df4b307ea47dd00cf530a25937a270aa57 100644 (file)
--- a/src/documentation/content/xdocs/status.xml
+++ b/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>

diff --git a/src/java/org/apache/poi/util/POILogFactory.java b/src/java/org/apache/poi/util/POILogFactory.java
index 74ea822a5f316c04a303ff9644a528050142501d..a9ce66f36b3273c4b2c2814bbc7176f5c7c0e15a 100644 (file)
--- a/src/java/org/apache/poi/util/POILogFactory.java
+++ b/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
-    private static Map          _loggers = new HashMap();;
-
+    /**
+     * Map of POILogger instances, with classes as keys
+     */
+    private static Map _loggers = new HashMap();;
 
     /**
-     * construct a POILogFactory.
+     * 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.
+     */
     private POILogFactory()
     {
     }
@@ -69,28 +80,48 @@ public class POILogFactory
     public static POILogger getLogger(final String cat)
     {
         POILogger logger = null;
+        
+        // If we haven't found out what logger to use yet,
+        //  then do so now
+        // Don't look it up until we're first asked, so
+        //  that our users can set the system property
+        //  between class loading and first use
+        if(_loggerClassName == null) {
+               try {
+                       _loggerClassName = System.getProperty("org.apache.poi.util.POILogger");
+               } 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;
+        }
 
-        if (_loggers.containsKey(cat))
-        {
+        
+        // Fetch the right logger for them, creating
+        //  it if that's required 
+        if (_loggers.containsKey(cat)) {
             logger = ( POILogger ) _loggers.get(cat);
-        }
-        else
-        {
-            try{
-              String loggerClassName = System.getProperty("org.apache.poi.util.POILogger");
-              Class loggerClass = Class.forName(loggerClassName);
+        } else {
+            try {
+              Class loggerClass = Class.forName(_loggerClassName);
               logger = ( POILogger ) loggerClass.newInstance();
+              logger.initialize(cat);
+            } catch(Exception e) {
+              // Give up and use the null logger
+              logger = _nullLogger;
             }
-            catch(Exception e){
-            
-              logger = new NullLogger();
-            }
-            
-            logger.initialize(cat);
             
+            // Save for next time
             _loggers.put(cat, logger);
         }
         return logger;
     }
-        
-}   // end public class POILogFactory
+}   // end public class POILogFactory
\ No newline at end of file

diff --git a/src/testcases/org/apache/poi/poifs/storage/TestRawDataBlock.java b/src/testcases/org/apache/poi/poifs/storage/TestRawDataBlock.java
index 4c84f04b71a884d710e863f547fd7b8031bc040b..73ddcad54312b376929c13a6f54916270066a748 100644 (file)
--- a/src/testcases/org/apache/poi/poifs/storage/TestRawDataBlock.java
+++ b/src/testcases/org/apache/poi/poifs/storage/TestRawDataBlock.java
@@ -36,23 +36,23 @@ import junit.framework.*;
 public class TestRawDataBlock
     extends TestCase
 {
+       static {
+        // We always want to use our own
+        //  logger
+        System.setProperty(
+                       "org.apache.poi.util.POILogger",
+                       "org.apache.poi.util.DummyPOILogger"
+        );
+       }
 
     /**
      * Constructor TestRawDataBlock
      *
      * @param name
      */
-
     public TestRawDataBlock(String name)
     {
         super(name);
-        
-        // We always want to use our own
-        //  logger
-        System.setProperty(
-                       "org.apache.poi.util.POILogger",
-                       "org.apache.poi.util.DummyPOILogger"
-        );
     }
 
     /**

diff --git a/src/testcases/org/apache/poi/poifs/storage/TestRawDataBlockList.java b/src/testcases/org/apache/poi/poifs/storage/TestRawDataBlockList.java
index d151029762089e1634a46b4ff9fadddc9643bc93..0f65c0e8a4421ac4f4830f1616d374ca8c674888 100644 (file)
--- a/src/testcases/org/apache/poi/poifs/storage/TestRawDataBlockList.java
+++ b/src/testcases/org/apache/poi/poifs/storage/TestRawDataBlockList.java
@@ -36,23 +36,23 @@ import junit.framework.*;
 public class TestRawDataBlockList
     extends TestCase
 {
+       static {
+        // We always want to use our own
+        //  logger
+        System.setProperty(
+                       "org.apache.poi.util.POILogger",
+                       "org.apache.poi.util.DummyPOILogger"
+        );
+       }
 
     /**
      * Constructor TestRawDataBlockList
      *
      * @param name
      */
-
     public TestRawDataBlockList(String name)
     {
         super(name);
-        
-        // We always want to use our own
-        //  logger
-        System.setProperty(
-                       "org.apache.poi.util.POILogger",
-                       "org.apache.poi.util.DummyPOILogger"
-        );
     }
 
     /**
@@ -60,7 +60,6 @@ public class TestRawDataBlockList
      *
      * @exception IOException
      */
-
     public void testNormalConstructor()
         throws IOException
     {