You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by oh...@apache.org on 2005/11/06 19:58:19 UTC

svn commit: r331135 - in /jakarta/commons/proper/configuration/trunk: src/java/org/apache/commons/configuration/AbstractFileConfiguration.java src/test/org/apache/commons/configuration/TestPropertiesConfiguration.java xdocs/changes.xml

Author: oheger
Date: Sun Nov  6 10:57:39 2005
New Revision: 331135

URL: http://svn.apache.org/viewcvs?rev=331135&view=rev
Log:
Prevented reentrance of AbstractFileConfiguration.reload(), which may cause infinite loops; related to issue 36665

Modified:
    jakarta/commons/proper/configuration/trunk/src/java/org/apache/commons/configuration/AbstractFileConfiguration.java
    jakarta/commons/proper/configuration/trunk/src/test/org/apache/commons/configuration/TestPropertiesConfiguration.java
    jakarta/commons/proper/configuration/trunk/xdocs/changes.xml

Modified: jakarta/commons/proper/configuration/trunk/src/java/org/apache/commons/configuration/AbstractFileConfiguration.java
URL: http://svn.apache.org/viewcvs/jakarta/commons/proper/configuration/trunk/src/java/org/apache/commons/configuration/AbstractFileConfiguration.java?rev=331135&r1=331134&r2=331135&view=diff
==============================================================================
--- jakarta/commons/proper/configuration/trunk/src/java/org/apache/commons/configuration/AbstractFileConfiguration.java (original)
+++ jakarta/commons/proper/configuration/trunk/src/java/org/apache/commons/configuration/AbstractFileConfiguration.java Sun Nov  6 10:57:39 2005
@@ -700,20 +700,29 @@
     {
         synchronized (reloadLock)
         {
-            if (noReload == 0 && strategy.reloadingRequired())
+            if (noReload == 0)
             {
                 try
                 {
-                    clear();
-                    load();
+                    enterNoReload(); // avoid reentrant calls
 
-                    // notify the strategy
-                    strategy.reloadingPerformed();
+                    if (strategy.reloadingRequired())
+                    {
+                        clear();
+                        load();
+
+                        // notify the strategy
+                        strategy.reloadingPerformed();
+                    }
                 }
                 catch (Exception e)
                 {
                     e.printStackTrace();
                     // todo rollback the changes if the file can't be reloaded
+                }
+                finally
+                {
+                    exitNoReload();
                 }
             }
         }

Modified: jakarta/commons/proper/configuration/trunk/src/test/org/apache/commons/configuration/TestPropertiesConfiguration.java
URL: http://svn.apache.org/viewcvs/jakarta/commons/proper/configuration/trunk/src/test/org/apache/commons/configuration/TestPropertiesConfiguration.java?rev=331135&r1=331134&r2=331135&view=diff
==============================================================================
--- jakarta/commons/proper/configuration/trunk/src/test/org/apache/commons/configuration/TestPropertiesConfiguration.java (original)
+++ jakarta/commons/proper/configuration/trunk/src/test/org/apache/commons/configuration/TestPropertiesConfiguration.java Sun Nov  6 10:57:39 2005
@@ -25,6 +25,8 @@
 import java.util.Iterator;
 import java.util.List;
 
+import org.apache.commons.configuration.reloading.FileChangedReloadingStrategy;
+
 import junit.framework.TestCase;
 
 /**
@@ -417,5 +419,22 @@
                 + EOL + EOL) == 0);
         assertTrue("Property could not be found", content
                 .indexOf("prop = value" + EOL) > 0);
+    }
+    
+    /**
+     * Tests what happens if a reloading strategy's <code>reloadingRequired()</code>
+     * implementation accesses methods of the configuration that in turn cause a reload.
+     */
+    public void testReentrantReload()
+    {
+        conf.setProperty("shouldReload", Boolean.FALSE);
+        conf.setReloadingStrategy(new FileChangedReloadingStrategy()
+        {
+            public boolean reloadingRequired()
+            {
+                return configuration.getBoolean("shouldReload");
+            }
+        });
+        assertFalse("Property has wrong value", conf.getBoolean("shouldReload"));
     }
 }

Modified: jakarta/commons/proper/configuration/trunk/xdocs/changes.xml
URL: http://svn.apache.org/viewcvs/jakarta/commons/proper/configuration/trunk/xdocs/changes.xml?rev=331135&r1=331134&r2=331135&view=diff
==============================================================================
--- jakarta/commons/proper/configuration/trunk/xdocs/changes.xml (original)
+++ jakarta/commons/proper/configuration/trunk/xdocs/changes.xml Sun Nov  6 10:57:39 2005
@@ -23,6 +23,11 @@
   <body>
 
     <release version="1.2-dev" date="in SVN">
+      <action dev="oheger" type="update" issue="36665">
+        The reload() method in AbstractFileConfiguration was updated to prevent
+        reentrant invocation, which may be caused by some methods when they
+        are called during a reloading operation.
+      </action>
       <action dev="ebourg, oheger" type="update">
         AbstractHierarchicalFileConfiguration, a new base class for file based
         hierarchical configurations, was introduced. XMLConfiguration now



---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org