svn commit: r1623862 - in /commons/proper/configuration/trunk/src: main/java/org/apache/commons/configuration/SystemConfiguration.java test/java/org/apache/commons/configuration/TestSystemConfiguration.java

[oh...@apache.org]

Author: oheger
Date: Tue Sep  9 17:56:29 2014
New Revision: 1623862

URL: http://svn.apache.org/r1623862
Log:
[CONFIGURATION-570] Alternative implementation of SystemConfiguration.getKeys().

In order to avoid ConcurrentModificationExceptions when iterating over the
keys of a SystemConfiguration, the iterator returned for the keys is now a
snapshot.

Modified:
    commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration/SystemConfiguration.java
    commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration/TestSystemConfiguration.java

Modified: commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration/SystemConfiguration.java
URL: http://svn.apache.org/viewvc/commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration/SystemConfiguration.java?rev=1623862&r1=1623861&r2=1623862&view=diff
==============================================================================
--- commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration/SystemConfiguration.java (original)
+++ commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration/SystemConfiguration.java Tue Sep  9 17:56:29 2014
@@ -104,4 +104,14 @@ public class SystemConfiguration extends
             System.setProperty(key, value);
         }
     }
+
+    /**
+     * {@inheritDoc} This implementation returns a snapshot of the keys in the
+     * system properties. If another thread modifies system properties concurrently,
+     * these changes are not reflected by the iterator returned by this method.
+     */
+    @Override
+    protected Iterator<String> getKeysInternal() {
+        return System.getProperties().stringPropertyNames().iterator();
+    }
 }

Modified: commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration/TestSystemConfiguration.java
URL: http://svn.apache.org/viewvc/commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration/TestSystemConfiguration.java?rev=1623862&r1=1623861&r2=1623862&view=diff
==============================================================================
--- commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration/TestSystemConfiguration.java (original)
+++ commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration/TestSystemConfiguration.java Tue Sep  9 17:56:29 2014
@@ -22,7 +22,9 @@ import static org.junit.Assert.assertTru
 
 import java.io.File;
 import java.io.IOException;
+import java.util.Iterator;
 import java.util.Properties;
+import java.util.concurrent.atomic.AtomicBoolean;
 
 import org.apache.commons.configuration.ex.ConfigurationException;
 import org.apache.commons.configuration.io.FileHandler;
@@ -92,4 +94,61 @@ public class TestSystemConfiguration
         assertEquals("System property not changed", "true",
                 System.getProperty(testProperty));
     }
+
+    /**
+     * Tests an append operation with a system configuration while system
+     * properties are modified from another thread. This is related to
+     * CONFIGURATION-570.
+     */
+    @Test
+    public void testAppendWhileConcurrentAccess() throws InterruptedException
+    {
+        final AtomicBoolean stop = new AtomicBoolean();
+        final String property =
+                SystemConfiguration.class.getName() + ".testProperty";
+        Thread t = new Thread()
+        {
+            @Override
+            public void run()
+            {
+                boolean setValue = true;
+                while (!stop.get())
+                {
+                    if (setValue)
+                    {
+                        System.setProperty(property, "true");
+                    }
+                    else
+                    {
+                        System.clearProperty(property);
+                    }
+                    setValue = !setValue;
+                }
+            }
+        };
+        try
+        {
+            t.start();
+
+            SystemConfiguration config = new SystemConfiguration();
+            PropertiesConfiguration props = new PropertiesConfiguration();
+            props.append(config);
+
+            stop.set(true);
+            t.join();
+            for (Iterator<String> keys = config.getKeys(); keys.hasNext();)
+            {
+                String key = keys.next();
+                if (!property.equals(key))
+                {
+                    assertEquals("Wrong value for " + key,
+                            config.getString(key), props.getString(key));
+                }
+            }
+        }
+        finally
+        {
+            System.clearProperty(property);
+        }
+    }
 }