Hi,
Occasionally we see incorrect translations of documents in the repository to
external urls. The same SitemenuLinkRewriterTranformer should never be used
by seperate threads, so the cause is probably in the
SiteMenuLinkRewriterImpl. One of the incorrect translations is for a
'perfect match' (location is the same as the datasource), so the error
should be between lines 164-200 of the SiteMenuLinkRewriterImpl. In that
code range I noticed a fairly classic multi-threading bug (roughly speaking,
it is 'double-checked locking') on line 168:
if (!(cache.containsKey(cacheKeyList) && cache.containsKey(cacheKeyMap))) {
initialize(host);
}
The initialize method is synchronized, but since the check isn't, a later
thread can use a semi-initialized cache, while an earlier thread is still in
the initialize method. A partial solution to this problem might be to place
a synchronize-block (locking the cache) around the code above (removing the
synchronization from the initialize method), which should prevent this
problem, would prevent duplicate initializations and would block other
threads (who are already translating) from seeing an partially initialized
cache (since the cache has synchronized methods, so cache.get will block
during the initialize). Of course, threads still might have to deal with a
cache that empties and/or get filled half-way through a translate, but
solving this requires a lot more synchronization.
I expect that the performance penalty of this change will be minimal. Do you
agree that this might be a good change?
Regards,
Wouter Zelle
********************************************
Hippocms-dev: Hippo CMS 6 development public mailinglist
Searchable archives can be found at:
MarkMail: http://hippocms-dev.markmail.org
Nabble: http://www.nabble.com/Hippo-CMS-f26633.html
