Hi Wouter, Thanks for your analysis. I'm not that familiar with the SitemenuLinkRewriterTranformer and it has been a long time since I had a look at the code. It sounds like there is something going wrong in there. I agree with your analysis and proposed (line of) change. Could you create an initial patch?
Regards, Bart On Thu, Mar 11, 2010 at 12:40 PM, Wouter Zelle <[email protected]> wrote: > 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 > > -- Hippo B.V. - Amsterdam Oosteinde 11, 1017 WT, Amsterdam, +31(0)20-5224466 Hippo USA Inc. - San Francisco 101 H Street, Suite Q, Petaluma CA, 94952-3329, +1 (707) 773-4646 ----------------------------------------------------------------- http://www.onehippo.com - [email protected] ----------------------------------------------------------------- ******************************************** 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
