On Nov 7, 2008, at 5:34 PM, Sergiu Dumitriu wrote:
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?
The getTranslation() method containing the synchronized() block.
-Vincent
> 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);
>> + }
>> }
>> }
>> }