Vincent Massol wrote:

Sergiu,

I don't understand this. You're doing a synchronization on an object that's recreated every time the method is called so it'll always have a different location in memory and there's no risk of contention.

Which method are you talking about?

On Nov 6, 2008, at 7:38 AM, sdumitriu (SVN) wrote:

...

Author: sdumitriu Date: 2008-11-06 07:38:49 +0100 (Thu, 06 Nov 2008) New Revision: 14000

Modified: platform/core/trunk/xwiki-localization/src/main/java/org/xwiki/ localization/internal/PulledDocumentsBundle.java platform/core/trunk/xwiki-localization/src/main/java/org/xwiki/ localization/internal/PulledFilesBundle.java platform/core/trunk/xwiki-localization/src/main/java/org/xwiki/ localization/internal/StaticDocumentsBundle.java Log: XWIKI-2811: Possible threading issues in the localization component Fixed.

Modified: platform/core/trunk/xwiki-localization/src/main/java/org/ xwiki/localization/internal/PulledDocumentsBundle.java =================================================================== --- platform/core/trunk/xwiki-localization/src/main/java/org/xwiki/ localization/internal/PulledDocumentsBundle.java 2008-11-06 06:31:12 UTC (rev 13999) +++ platform/core/trunk/xwiki-localization/src/main/java/org/xwiki/ localization/internal/PulledDocumentsBundle.java 2008-11-06 06:38:49 UTC (rev 14000) @@ -63,17 +63,19 @@ List<String> documentNames = (List<String>) this.execution.getContext().getProperty(PULLED_CONTEXT_KEY); if (documentNames != null) { Properties props; - for (String documentName : documentNames) { - try { - // The document names should contain the wiki prefix already. - props = getDocumentBundle(documentName, language); - if (props.containsKey(key)) { - translation = props.getProperty(key); - // The first translation found is returned. - break; + synchronized (documentNames) { + for (String documentName : documentNames) { + try { + // The document names should contain the wiki prefix already. + props = getDocumentBundle(documentName, language); + if (props.containsKey(key)) { + translation = props.getProperty(key); + // The first translation found is returned. + break; + } + } catch (Exception ex) { + getLogger().warn("Cannot load document bundle: [{0}]", documentName); } - } catch (Exception ex) { - getLogger().warn("Cannot load document bundle: [{0}]", documentName); } } } @@ -102,7 +104,9 @@ documentNames = new ArrayList<String>();

this.execution.getContext().setProperty(PULLED_CONTEXT_KEY, documentNames); } - documentNames.add(documentName); + synchronized (documentNames) { + documentNames.add(documentName); + } } }

Modified: platform/core/trunk/xwiki-localization/src/main/java/org/ xwiki/localization/internal/PulledFilesBundle.java =================================================================== --- platform/core/trunk/xwiki-localization/src/main/java/org/xwiki/ localization/internal/PulledFilesBundle.java 2008-11-06 06:31:12 UTC (rev 13999) +++ platform/core/trunk/xwiki-localization/src/main/java/org/xwiki/ localization/internal/PulledFilesBundle.java 2008-11-06 06:38:49 UTC (rev 14000) @@ -54,16 +54,18 @@ List<String> fileNames = (List<String>) this.execution.getContext().getProperty(PULLED_CONTEXT_KEY); if (fileNames != null) { Properties props; - for (String fileName : fileNames) { - try { - props = getFileBundle(fileName, language); - if (props.containsKey(key)) { - translation = props.getProperty(key); - // The first translation found is returned. - break; + synchronized (fileNames) { + for (String fileName : fileNames) { + try { + props = getFileBundle(fileName, language); + if (props.containsKey(key)) { + translation = props.getProperty(key); + // The first translation found is returned. + break; + } + } catch (Exception e) { + getLogger().warn("Cannot load resource bundle: [{0}]", fileName); } - } catch (Exception e) { - getLogger().warn("Cannot load resource bundle: [{0}]", fileName); } } } @@ -86,7 +88,9 @@ fileNames = new ArrayList<String>();

this.execution.getContext().setProperty(PULLED_CONTEXT_KEY, fileNames); } - fileNames.add(bundleLocation); + synchronized (fileNames) { + fileNames.add(bundleLocation); + } } } }

Modified: platform/core/trunk/xwiki-localization/src/main/java/org/ xwiki/localization/internal/StaticDocumentsBundle.java =================================================================== --- platform/core/trunk/xwiki-localization/src/main/java/org/xwiki/ localization/internal/StaticDocumentsBundle.java 2008-11-06 06:31:12 UTC (rev 13999) +++ platform/core/trunk/xwiki-localization/src/main/java/org/xwiki/ localization/internal/StaticDocumentsBundle.java 2008-11-06 06:38:49 UTC (rev 14000) @@ -111,7 +111,9 @@ String oldBundleNames = StringUtils.join(this.staticBundleNames.remove(wiki), JOIN_SEPARATOR); String newBundleNames = StringUtils.join(this.getStaticDocumentBundles(wiki), JOIN_SEPARATOR); if (!StringUtils.equals(oldBundleNames, newBundleNames)) { - this.staticBundles.remove(wiki); + synchronized (this.staticBundles) { + this.staticBundles.remove(wiki); + } } } }

-- Sergiu Dumitriu http://purl.org/net/sergiu/