Re: [xwiki-devs] [xwiki-notifications] r14000 - platform/core/trunk/xwiki-localization/src/main/java/org/xwiki/localization/internal

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.

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); > + } > } > } > }