You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by oh...@apache.org on 2013/06/03 22:26:19 UTC
svn commit: r1489155 - in /commons/proper/configuration/trunk/src:
main/java/org/apache/commons/configuration/ site/xdoc/userguide/
test/java/org/apache/commons/configuration/
Author: oheger
Date: Mon Jun 3 20:26:18 2013
New Revision: 1489155
URL: http://svn.apache.org/r1489155
Log:
Fixed another race condition with DynamicCombinedConfiguration.
Now all child configurations share a single Synchronizer with the parent
configuration. This ensures proper synchronization.
Modified:
commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration/DynamicCombinedConfiguration.java
commons/proper/configuration/trunk/src/site/xdoc/userguide/howto_concurrency.xml
commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration/SynchronizerTestImpl.java
commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration/TestDynamicCombinedConfiguration.java
Modified: commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration/DynamicCombinedConfiguration.java
URL: http://svn.apache.org/viewvc/commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration/DynamicCombinedConfiguration.java?rev=1489155&r1=1489154&r2=1489155&view=diff
==============================================================================
--- commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration/DynamicCombinedConfiguration.java (original)
+++ commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration/DynamicCombinedConfiguration.java Mon Jun 3 20:26:18 2013
@@ -899,7 +899,8 @@ public class DynamicCombinedConfiguratio
endWrite();
}
- super.beginRead(optimize);
+ // This actually uses our own synchronizer
+ cch.getCurrentConfiguration().beginRead(optimize);
}
/**
@@ -929,7 +930,7 @@ public class DynamicCombinedConfiguratio
@Override
protected void endRead()
{
- super.endRead();
+ currentConfig.get().getCurrentConfiguration().endRead();
releaseLock();
}
@@ -1032,7 +1033,7 @@ public class DynamicCombinedConfiguratio
config.addConfiguration(data.getConfiguration(), data.getName(),
data.getAt());
}
- config.setSynchronizer(ConfigurationUtils.cloneSynchronizer(getSynchronizer()));
+ config.setSynchronizer(getSynchronizer());
}
/**
Modified: commons/proper/configuration/trunk/src/site/xdoc/userguide/howto_concurrency.xml
URL: http://svn.apache.org/viewvc/commons/proper/configuration/trunk/src/site/xdoc/userguide/howto_concurrency.xml?rev=1489155&r1=1489154&r2=1489155&view=diff
==============================================================================
--- commons/proper/configuration/trunk/src/site/xdoc/userguide/howto_concurrency.xml (original)
+++ commons/proper/configuration/trunk/src/site/xdoc/userguide/howto_concurrency.xml Mon Jun 3 20:26:18 2013
@@ -252,11 +252,11 @@ finally
which has not been encountered before, a new <code>CombinedConfiguration</code>
object is created. Here again it turns out that even a read access to a
<code>DynamicCombinedConfiguration</code> may cause internal state
- changes which require a write lock to be held. The <code>Synchronizer</code>
- objects used by the child combined configurations are not under the
- control of client code. The <code>DynamicCombinedConfiguration</code>
- initializes each child configuration with a new <code>Synchronizer</code>
- that is a clone from its own.</li>
+ changes which require a write lock to be held. When creating a new
+ child combined configuration it is passed the <code>Synchronizer</code>
+ of the owning <code>DynamicCombinedConfiguration</code>; so there is
+ actually only a single <code>Synchronizer</code> controlling the
+ access to all involved configurations.</li>
</ul>
</p>
</subsection>
Modified: commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration/SynchronizerTestImpl.java
URL: http://svn.apache.org/viewvc/commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration/SynchronizerTestImpl.java?rev=1489155&r1=1489154&r2=1489155&view=diff
==============================================================================
--- commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration/SynchronizerTestImpl.java (original)
+++ commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration/SynchronizerTestImpl.java Mon Jun 3 20:26:18 2013
@@ -112,6 +112,14 @@ public class SynchronizerTestImpl implem
}
/**
+ * Clears the methods recorded so far.
+ */
+ public void clear()
+ {
+ methods.setLength(0);
+ }
+
+ /**
* Generates a string with expected methods from the given array.
*
* @param expMethods the array with expected methods
Modified: commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration/TestDynamicCombinedConfiguration.java
URL: http://svn.apache.org/viewvc/commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration/TestDynamicCombinedConfiguration.java?rev=1489155&r1=1489154&r2=1489155&view=diff
==============================================================================
--- commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration/TestDynamicCombinedConfiguration.java (original)
+++ commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration/TestDynamicCombinedConfiguration.java Mon Jun 3 20:26:18 2013
@@ -107,6 +107,22 @@ public class TestDynamicCombinedConfigur
}
/**
+ * Tests whether a configuration can be updated.
+ */
+ @Test
+ public void testUpdateConfiguration() throws ConfigurationException
+ {
+ System.getProperties().remove("Id");
+ CombinedConfigurationBuilder builder =
+ new CombinedConfigurationBuilder();
+ builder.configure(Parameters.fileBased().setFile(MULTI_TENENT_FILE)
+ .setSynchronizer(new ReadWriteSynchronizer()));
+ CombinedConfiguration config = builder.getConfiguration();
+ config.getConfiguration(1).setProperty("rowsPerPage", "25");
+ assertEquals("Value not changed", "25", config.getString("rowsPerPage"));
+ }
+
+ /**
* Prepares a test for calling the Synchronizer. This method creates a test
* Synchronizer, installs it at the configuration and returns it.
*
@@ -115,10 +131,11 @@ public class TestDynamicCombinedConfigur
*/
private SynchronizerTestImpl prepareSynchronizerTest(Configuration config)
{
- config.lock(LockMode.READ);
- config.unlock(LockMode.READ); // ensure that root node is constructed
SynchronizerTestImpl sync = new SynchronizerTestImpl();
config.setSynchronizer(sync);
+ config.lock(LockMode.READ);
+ config.unlock(LockMode.READ); // ensure that root node is constructed
+ sync.clear();
return sync;
}