[ 
https://issues.apache.org/jira/browse/CONFIGURATION-390?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12759970#action_12759970
 ] 

Ralph Goers commented on CONFIGURATION-390:
-------------------------------------------

After a few more minutes of thought I think even this restriction won't solve 
the problem.  If two threads access the Combined Configuration, one may get 
through getRootNode and start working with it. The second thread will detect a 
reload and start into constructCombinedNode and that will screw up the thread 
that is working with it even though they will have different "combinedRoot" 
nodes, because the underlying Nodes from the configurations being combined are 
being messed with.

I can only think of two ways out of this:
1. Clone the underlying configurations. This is a really ugly solution as 
something like MultiFileHierachicalConfiguration won't be pretty to clone.
2. Synchronize the public methods that need access to the root node more or 
less the same as is needed in XMLConfiguration. Again, this can be optimized in 
2.0 by using Read/Write locks. I'd love to do this in 1.7 but I really don't 
want to introduce a dependency on util.concurrent. 

> AbstractHierarchicalFileConfiguration is not thread safe
> --------------------------------------------------------
>
>                 Key: CONFIGURATION-390
>                 URL: https://issues.apache.org/jira/browse/CONFIGURATION-390
>             Project: Commons Configuration
>          Issue Type: Bug
>          Components: File reloading
>    Affects Versions: 1.6
>            Reporter: Emmanuel Bourg
>             Fix For: 1.7
>
>         Attachments: commons-configuration-390.patch, configtest.tar.gz, 
> TestSuite.txt
>
>
> AbstractHierarchicalFileConfiguration doesn't implement the same locking 
> mechanism found in AbstractFileConfiguration. The consequence is that getting 
> a property while the configuration is being reloaded by another thread can 
> return an invalid result.
> This can be demonstrated by changing testDeadlockWithReload() in 
> TestCombinedConfiguration to use an XMLConfiguration instead of a 
> PropertiesConfiguration.
> Here is a reduced test case:
> {code:java}
> public void testConcurrentGetAndReload() throws Exception
> {
>     //final FileConfiguration config = new 
> PropertiesConfiguration("test.properties");
>     final FileConfiguration config = new XMLConfiguration("test.xml");
>     config.setReloadingStrategy(new FileAlwaysReloadingStrategy());
>     assertTrue("Property not found", config.getProperty("test.short") != 
> null);
>     new Thread()
>     {
>         public void run()
>         {
>             for (int i = 0; i < 1000; i++)
>             {
>                 config.reload();
>             }
>         }
>     }.start();
>     
>     for (int i = 0; i < 1000; i++)
>     {
>         assertTrue("Property not found", config.getProperty("test.short") != 
> null); // failure here
>     }
> }
> {code}
> The test doesn't always fail. It does about 50% of the time.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply via email to