[
https://issues.apache.org/jira/browse/CONFIGURATION-344?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Oliver Heger resolved CONFIGURATION-344.
----------------------------------------
Resolution: Fixed
Fix Version/s: 1.6
A fix was applied to trunk in revision 712401.
So far, accessing the root node of the combined configuration was synchronized
because it had to be re-constructed if something had changed. This fix removes
this synchronization and - to ensure proper visibility - adds the *volatile*
modifier to the root node field. If now multiple threads want to access the
combined configuration, there may be a race condition causing the root node to
be constructed multiple times. However, this does not impact the results of
queries or have other undesired side effects.
When testing I found out that reloading operations are indeed problematic when
multiple threads are involved. The unit test case uses a synthetic reloading
strategy that forces a reload on every access. {{AbstractFileConfiguration}}
overrides all access methods to ensure that {{reload()}} is invoked as in the
following example:
{code}
public Object getProperty(String key)
{
reload();
return super.getProperty(key);
}
{code}
If there are multiple concurrent reloads or property accesses in sequence - as
in the test I created -, the following race condition can happen:
* One thread executes {{reload()}} and then is going to enter {{getProperty()}}.
* Another thread wants to read a property and enters {{reload()}}. Because
{{reload()}} is guarded by a lock, the thread has to wait.
* As soon as the first thread leaves {{reload()}} the second can proceed and
initiates the reload operation.
* While trying to read a property the first thread sees an invalid state
because a reload operation is in progress.
To get my test running I changed the implementation of {{getProperty()}} in the
following way:
{code}
public Object getProperty(String key)
{
synchronized (reloadLock)
{
reload();
return super.getProperty(key);
}
}
{code}
So the whole property read operation is now guarded by the lock. To be on the
safe side this change would have to be applied for the other methods, too. But
I think, the architecture of the reloading implementation should better be
re-worked in the next major release. Note that it is very unlikely that
multiple reloads happen in short intervals because the available reloading
strategies have a refresh delay.
> Deadlock during refresh properties
> ----------------------------------
>
> Key: CONFIGURATION-344
> URL: https://issues.apache.org/jira/browse/CONFIGURATION-344
> Project: Commons Configuration
> Issue Type: Bug
> Components: Events & Notifications, File reloading
> Affects Versions: 1.5
> Reporter: Pavel Savara
> Assignee: Oliver Heger
> Priority: Critical
> Fix For: 1.6
>
>
> Hi
> Commons configurations get itself stuck in deadlock when refreshing
> properties using Managed reloading strategy. It seems to me it get stuck
> because of fireEvent in reload method. Another access grabs lock on
> synchronized (getNodeCombiner()) when trying to rebuild but Combined
> configuration is one of the listeners for event es well and it gets
> stuck when processing invalidate. Can anyone suggest quick fix please?
> Relevant information follows.
> Thanks
> Pavel
> Configuration:
> <configuration>
> <override>
> <system/>
> <properties fileName="gsxweb.properties" throwExceptionOnMissing="false"
> config-name="gsxweb" config-optional="false" listDelimiter="|">
> <reloadingStrategy
> config-class="org.apache.commons.configuration.reloading.ManagedReloadingStrategy"/>
>
> </properties>
> </override>
> </configuration>
> Our Reload code:
> int ln = combinedConfiguration.getNumberOfConfigurations();
> int reloaded = 0;
> for (int i = 0; i < ln; i++) {
> Configuration conf = combinedConfiguration.getConfiguration(i);
> if (conf instanceof PropertiesConfiguration) {
> ManagedReloadingStrategy strat = null;
> ReloadingStrategy strategy = ((PropertiesConfiguration)
> conf).getReloadingStrategy();
> //refresh if managed strategy
> if (strategy instanceof ManagedReloadingStrategy) {
> ((ManagedReloadingStrategy) strategy).refresh();
> //reload if file changed strategy
> } else if (strategy instanceof FileChangedReloadingStrategy)
> {
> ((PropertiesConfiguration) conf).reload();
> }
> reloaded++;
> }
> }
> Stack trace of deadlock threads
> Name: http-10980-1
> State: BLOCKED on
> [EMAIL PROTECTED] owned by:
> http-10980-6
> Total blocked: 154 Total waited: 2
> Stack trace:
> org.apache.commons.configuration.CombinedConfiguration.invalidate(CombinedConfiguration.java:474)
> org.apache.commons.configuration.CombinedConfiguration.configurationChanged(CombinedConfiguration.java:488)
> org.apache.commons.configuration.event.EventSource.fireEvent(EventSource.java:249)
> org.apache.commons.configuration.AbstractFileConfiguration.fireEvent(AbstractFileConfiguration.java:911)
> org.apache.commons.configuration.AbstractFileConfiguration.reload(AbstractFileConfiguration.java:828)
> - locked [EMAIL PROTECTED]
> org.apache.commons.configuration.AbstractFileConfiguration.isEmpty(AbstractFileConfiguration.java:927)
> org.apache.commons.configuration.reloading.ManagedReloadingStrategy.refresh(ManagedReloadingStrategy.java:91)
> com.gsx.properties.PropertyProviderImpl.reset(PropertyProviderImpl.java:203)
> - locked [EMAIL PROTECTED]
> org.apache.jsp.test.testPropertyProvider_jsp._jspService(testPropertyProvider_jsp.java:60)
> org.apache.jasper.runtime.HttpJspBase.service(HttpJspBase.java:70)
> javax.servlet.http.HttpServlet.service(HttpServlet.java:717)
> org.apache.jasper.servlet.JspServletWrapper.service(JspServletWrapper.java:374)
> org.apache.jasper.servlet.JspServlet.serviceJspFile(JspServlet.java:342)
> org.apache.jasper.servlet.JspServlet.service(JspServlet.java:267)
> javax.servlet.http.HttpServlet.service(HttpServlet.java:717)
> org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:290)
> Name: http-10980-6
> State: BLOCKED on [EMAIL PROTECTED] owned by: http-10980-1
> Total blocked: 115 Total waited: 2
> Stack trace:
> org.apache.commons.configuration.AbstractFileConfiguration.reload(AbstractFileConfiguration.java:814)
> org.apache.commons.configuration.AbstractFileConfiguration.getKeys(AbstractFileConfiguration.java:939)
> org.apache.commons.configuration.ConfigurationUtils.copy(ConfigurationUtils.java:139)
> org.apache.commons.configuration.ConfigurationUtils.convertToHierarchical(ConfigurationUtils.java:199)
> org.apache.commons.configuration.CombinedConfiguration
> $ConfigData.getTransformedRoot(CombinedConfiguration.java:794)
> org.apache.commons.configuration.CombinedConfiguration.constructCombinedNode(CombinedConfiguration.java:653)
> org.apache.commons.configuration.CombinedConfiguration.getRootNode(CombinedConfiguration.java:504)
> - locked
> [EMAIL PROTECTED]
> org.apache.commons.configuration.HierarchicalConfiguration.fetchNodeList(HierarchicalConfiguration.java:925)
> org.apache.commons.configuration.HierarchicalConfiguration.getProperty(HierarchicalConfiguration.java:327)
> org.apache.commons.configuration.CombinedConfiguration.getProperty(CombinedConfiguration.java:578)
> org.apache.commons.configuration.AbstractConfiguration.resolveContainerStore(AbstractConfiguration.java:1155)
> org.apache.commons.configuration.AbstractConfiguration.getString(AbstractConfiguration.java:1034)
> org.apache.jsp.test.testPropertyProvider_jsp._jspService(testPropertyProvider_jsp.java:69)
> org.apache.jasper.runtime.HttpJspBase.service(HttpJspBase.java:70)
> javax.servlet.http.HttpServlet.service(HttpServlet.java:717)
> org.apache.jasper.servlet.JspServletWrapper.service(JspServletWrapper.java:374)
> org.apache.jasper.servlet.JspServlet.serviceJspFile(JspServlet.java:342)
> org.apache.jasper.servlet.JspServlet.service(JspServlet.java:267)
> javax.servlet.http.HttpServlet.service(HttpServlet.java:717)
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
