myfaces-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Leonardo Uribe (JIRA)" <...@myfaces.apache.org>
Subject [jira] [Resolved] (MYFACES-3705) Concurrency "feature" in FaceletCacheImpl
Date Mon, 22 Apr 2013 19:29:16 GMT

     [ https://issues.apache.org/jira/browse/MYFACES-3705?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]

Leonardo Uribe resolved MYFACES-3705.
-------------------------------------

       Resolution: Fixed
    Fix Version/s: 2.1.12
    
> Concurrency "feature" in FaceletCacheImpl
> -----------------------------------------
>
>                 Key: MYFACES-3705
>                 URL: https://issues.apache.org/jira/browse/MYFACES-3705
>             Project: MyFaces Core
>          Issue Type: Bug
>          Components: General
>    Affects Versions: 2.1.0
>         Environment: All
>            Reporter: Pasi Salminen
>            Assignee: Leonardo Uribe
>            Priority: Trivial
>             Fix For: 2.1.12
>
>   Original Estimate: 1h
>  Remaining Estimate: 1h
>
> I'm implementing my own FaceletCache which is decorating org.apache.myfaces.view.facelets.impl.FaceletCacheImpl
by adding my own caching policy. When I was studying the code I'm decorating, I noticed that
scrictly speaking it was not behaving. The problem lies in this code snippet (and the same
for metadata facelets):
>             if (_refreshPeriod != NO_CACHE_DELAY)
>             {
>                 Map<String, DefaultFacelet> newLoc = new HashMap<String, DefaultFacelet>(_facelets);
>                 newLoc.put(key, f);
>                 _facelets = newLoc;
>             }
> First of all, multiple concurrent modifications of _facelets map may cause lost updates.
Think what happens when thread one copies the hashmap, updates local copy but before it sets
the reference, thread b does the same. One update is now lost. In reality, the number of concurrent
threads and number of lost updates may be much larger. The second thing is that the new reference
set to _facelets is not quaranteed to be visible to other threads due to missing synchronization.
To prove my concerns, I created a small test bench which proved my doubts and was able to
show both lost updates and visibility problem. When I modified the map to be ConcurrentHashMap
and just used put-method all problems vanished. So instead of
>                 Map<String, DefaultFacelet> newLoc = new HashMap<String, DefaultFacelet>(_facelets);
>                 newLoc.put(key, f);
>                 _facelets = newLoc;
> I used
>                 _facelets.put( key,f );
> I know it may not be a problem, possibly just causing multiple loads of same resource,
but still I don't feel comfortable with the code behaving concurrency-wise incorrectly.
> BR, Paci

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

Mime
View raw message