[ 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
> org.apache.commons.configuration.tree.OverrideCombiner@8511bb 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 java.lang.Object@127e34c
> 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 java.lang.Class@109bcda
> 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 java.lang.Object@127e34c 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
> org.apache.commons.configuration.tree.OverrideCombiner@8511bb
> 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.
|