commons-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ohe...@apache.org
Subject svn commit: r712401 - in /commons/proper/configuration/trunk: src/java/org/apache/commons/configuration/ src/test/org/apache/commons/configuration/ xdocs/
Date Sat, 08 Nov 2008 15:29:57 GMT
Author: oheger
Date: Sat Nov  8 07:29:56 2008
New Revision: 712401

URL: http://svn.apache.org/viewvc?rev=712401&view=rev
Log:
CONFIGURATION-344: Reduced synchronization in CombinedConfiguration when accessing and constructing
the combined root node. This fixes a potential deadlock related to reload operations in child
configurations.

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

Modified: commons/proper/configuration/trunk/src/java/org/apache/commons/configuration/AbstractFileConfiguration.java
URL: http://svn.apache.org/viewvc/commons/proper/configuration/trunk/src/java/org/apache/commons/configuration/AbstractFileConfiguration.java?rev=712401&r1=712400&r2=712401&view=diff
==============================================================================
--- commons/proper/configuration/trunk/src/java/org/apache/commons/configuration/AbstractFileConfiguration.java
(original)
+++ commons/proper/configuration/trunk/src/java/org/apache/commons/configuration/AbstractFileConfiguration.java
Sat Nov  8 07:29:56 2008
@@ -920,8 +920,11 @@
 
     public Object getProperty(String key)
     {
-        reload();
-        return super.getProperty(key);
+        synchronized (reloadLock)
+        {
+            reload();
+            return super.getProperty(key);
+        }
     }
 
     public boolean isEmpty()

Modified: commons/proper/configuration/trunk/src/java/org/apache/commons/configuration/CombinedConfiguration.java
URL: http://svn.apache.org/viewvc/commons/proper/configuration/trunk/src/java/org/apache/commons/configuration/CombinedConfiguration.java?rev=712401&r1=712400&r2=712401&view=diff
==============================================================================
--- commons/proper/configuration/trunk/src/java/org/apache/commons/configuration/CombinedConfiguration.java
(original)
+++ commons/proper/configuration/trunk/src/java/org/apache/commons/configuration/CombinedConfiguration.java
Sat Nov  8 07:29:56 2008
@@ -193,7 +193,7 @@
     private NodeCombiner nodeCombiner;
 
     /** Stores the combined root node. */
-    private ConfigurationNode combinedRoot;
+    private volatile ConfigurationNode combinedRoot;
 
     /** Stores a list with the contained configurations. */
     private List configurations;
@@ -509,10 +509,7 @@
      */
     public void invalidate()
     {
-        synchronized (getNodeCombiner()) // use combiner as lock
-        {
-            combinedRoot = null;
-        }
+        combinedRoot = null;
         fireEvent(EVENT_COMBINED_INVALIDATE, null, null, false);
     }
 
@@ -540,14 +537,11 @@
      */
     public ConfigurationNode getRootNode()
     {
-        synchronized (getNodeCombiner())
+        if (combinedRoot == null)
         {
-            if (combinedRoot == null)
-            {
-                combinedRoot = constructCombinedNode();
-            }
-            return combinedRoot;
+            combinedRoot = constructCombinedNode();
         }
+        return combinedRoot;
     }
 
     /**

Modified: commons/proper/configuration/trunk/src/test/org/apache/commons/configuration/TestCombinedConfiguration.java
URL: http://svn.apache.org/viewvc/commons/proper/configuration/trunk/src/test/org/apache/commons/configuration/TestCombinedConfiguration.java?rev=712401&r1=712400&r2=712401&view=diff
==============================================================================
--- commons/proper/configuration/trunk/src/test/org/apache/commons/configuration/TestCombinedConfiguration.java
(original)
+++ commons/proper/configuration/trunk/src/test/org/apache/commons/configuration/TestCombinedConfiguration.java
Sat Nov  8 07:29:56 2008
@@ -24,6 +24,7 @@
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Iterator;
+import java.util.NoSuchElementException;
 import java.util.Set;
 
 import junit.framework.Assert;
@@ -661,6 +662,54 @@
     }
 
     /**
+     * Tests whether reload operations can cause a deadlock when the combined
+     * configuration is accessed concurrently. This test is related to
+     * CONFIGURATION-344.
+     */
+    public void testDeadlockWithReload() throws ConfigurationException,
+            InterruptedException
+    {
+        final PropertiesConfiguration child = new PropertiesConfiguration(
+                "test.properties");
+        child.setReloadingStrategy(new FileAlwaysReloadingStrategy());
+        config.addConfiguration(child);
+        final int count = 1000;
+
+        class ReloadThread extends Thread
+        {
+            boolean error = false;
+
+            public void run()
+            {
+                for (int i = 0; i < count && !error; i++)
+                {
+                    try
+                    {
+                        if (!child.getBoolean("configuration.loaded"))
+                        {
+                            error = true;
+                        }
+                    }
+                    catch (NoSuchElementException nsex)
+                    {
+                        error = true;
+                    }
+                }
+            }
+        }
+
+        ReloadThread reloadThread = new ReloadThread();
+        reloadThread.start();
+        for (int i = 0; i < count; i++)
+        {
+            assertEquals("Wrong value of combined property", 10, config
+                    .getInt("test.integer"));
+        }
+        reloadThread.join();
+        assertFalse("Failure in thread", reloadThread.error);
+    }
+
+    /**
      * Helper method for writing a file. The file is also added to a list and
      * will be deleted in teadDown() automatically.
      *

Modified: commons/proper/configuration/trunk/xdocs/changes.xml
URL: http://svn.apache.org/viewvc/commons/proper/configuration/trunk/xdocs/changes.xml?rev=712401&r1=712400&r2=712401&view=diff
==============================================================================
--- commons/proper/configuration/trunk/xdocs/changes.xml (original)
+++ commons/proper/configuration/trunk/xdocs/changes.xml Sat Nov  8 07:29:56 2008
@@ -34,6 +34,12 @@
         configuration nodes for properties with multiple values. This
         improves compatibility with queries.
       </action>
+      <action dev="oheger" type="fix" issue="CONFIGURATION-344">
+        CombinedConfiguration could cause a deadlock when it was accessed while
+        concurrently a reload of one of its child configuration happened. This
+        was fixed by reducing synchronization where it is not strictly
+        necessary.
+      </action>
       <action dev="oheger" type="fix" issue="CONFIGURATION-341">
         The "force reload check" mechanism of CombinedConfiguration now also
         works with sub configurations created by configurationAt().



Mime
View raw message