commons-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Oliver Heger (JIRA)" <j...@apache.org>
Subject [jira] Resolved: (CONFIGURATION-344) Deadlock during refresh properties
Date Sat, 08 Nov 2008 15:54:44 GMT

     [ 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.


Mime
View raw message