You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ofbiz.apache.org by jl...@apache.org on 2013/11/13 23:04:34 UTC

svn commit: r1541746 - /ofbiz/trunk/applications/product/src/org/ofbiz/product/product/ProductEvents.java

Author: jleroux
Date: Wed Nov 13 22:04:33 2013
New Revision: 1541746

URL: http://svn.apache.org/r1541746
Log:
Fix an issue introduced with r1541641, by finally using synchronized that I tried to avoid

Modified:
    ofbiz/trunk/applications/product/src/org/ofbiz/product/product/ProductEvents.java

Modified: ofbiz/trunk/applications/product/src/org/ofbiz/product/product/ProductEvents.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/product/src/org/ofbiz/product/product/ProductEvents.java?rev=1541746&r1=1541745&r2=1541746&view=diff
==============================================================================
--- ofbiz/trunk/applications/product/src/org/ofbiz/product/product/ProductEvents.java (original)
+++ ofbiz/trunk/applications/product/src/org/ofbiz/product/product/ProductEvents.java Wed Nov 13 22:04:33 2013
@@ -24,6 +24,7 @@ import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
 import javax.servlet.http.HttpServletRequest;
@@ -1186,8 +1187,11 @@ public class ProductEvents {
         if (UtilValidate.isNotEmpty(productId) && UtilValidate.isNotEmpty(productTags)) {
             List<String> matchList = FastList.newInstance();
             Pattern regex = Pattern.compile("[^\\s\"']+|\"([^\"]*)\"|'([^']*)'");
-            while (regex.matcher(productTags).find()) {
-                matchList.add(regex.matcher(productTags).group().replace("'", ""));
+            Matcher regexMatcher = regex.matcher(productTags);
+            synchronized (regexMatcher) {
+                while (regexMatcher.find()) {
+                    matchList.add(regexMatcher.group().replace("'", ""));
+                }                
             }
             
             GenericValue userLogin = null;



Re: svn commit: r1541746 - /ofbiz/trunk/applications/product/src/org/ofbiz/product/product/ProductEven ts.java

Posted by Jacques Le Roux <ja...@les7arts.com>.
Oops, you are right. I focused on the Matcher class not being thread safe.
But indeed local variables (thread own stack) are always thread safe

I reverted both r1541641 and r1541746 at r1541894  

This also answer to Jacopo

Jacques

On Thursday, November 14, 2013 1:03 PM Adrian Crum <ad...@sandglass-software.com> wrote:
> This doesn't make sense. The regexMatcher object and the matchList
> object are both local to the current thread. Why do we need to
> synchronize them?
> 
> Adrian Crum
> Sandglass Software
> www.sandglass-software.com
> 
> On 11/13/2013 5:04 PM, jleroux@apache.org wrote:
>> Author: jleroux
>> Date: Wed Nov 13 22:04:33 2013
>> New Revision: 1541746
>> 
>> URL: http://svn.apache.org/r1541746
>> Log:
>> Fix an issue introduced with r1541641, by finally using synchronized that I tried to avoid
>> 
>> Modified:
>>      ofbiz/trunk/applications/product/src/org/ofbiz/product/product/ProductEvents.java
>> 
>> Modified: ofbiz/trunk/applications/product/src/org/ofbiz/product/product/ProductEvents.java
>> URL:
>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/product/src/org/ofbiz/product/product/ProductEvents.java?rev=1541746&r1=1541745&r2=1541746&view=diff
>> ============================================================================== ---
>> ofbiz/trunk/applications/product/src/org/ofbiz/product/product/ProductEvents.java (original) +++
>> ofbiz/trunk/applications/product/src/org/ofbiz/product/product/ProductEvents.java Wed Nov 13 22:04:33 2013 @@ -24,6 +24,7 @@
>>   import java.util.Iterator; import java.util.List;
>>   import java.util.Map;
>>   import java.util.Set;
>> +import java.util.regex.Matcher;
>>   import java.util.regex.Pattern;
>> 
>>   import javax.servlet.http.HttpServletRequest;
>> @@ -1186,8 +1187,11 @@ public class ProductEvents {
>>           if (UtilValidate.isNotEmpty(productId) && UtilValidate.isNotEmpty(productTags)) {
>>               List<String> matchList = FastList.newInstance();
>>               Pattern regex = Pattern.compile("[^\\s\"']+|\"([^\"]*)\"|'([^']*)'");
>> -            while (regex.matcher(productTags).find()) {
>> -                matchList.add(regex.matcher(productTags).group().replace("'", ""));
>> +            Matcher regexMatcher = regex.matcher(productTags);
>> +            synchronized (regexMatcher) {
>> +                while (regexMatcher.find()) {
>> +                    matchList.add(regexMatcher.group().replace("'", ""));
>> +                }
>>               }
>> 
>>               GenericValue userLogin = null;

Re: svn commit: r1541746 - /ofbiz/trunk/applications/product/src/org/ofbiz/product/product/ProductEvents.java

Posted by Adrian Crum <ad...@sandglass-software.com>.
This doesn't make sense. The regexMatcher object and the matchList 
object are both local to the current thread. Why do we need to 
synchronize them?

Adrian Crum
Sandglass Software
www.sandglass-software.com

On 11/13/2013 5:04 PM, jleroux@apache.org wrote:
> Author: jleroux
> Date: Wed Nov 13 22:04:33 2013
> New Revision: 1541746
>
> URL: http://svn.apache.org/r1541746
> Log:
> Fix an issue introduced with r1541641, by finally using synchronized that I tried to avoid
>
> Modified:
>      ofbiz/trunk/applications/product/src/org/ofbiz/product/product/ProductEvents.java
>
> Modified: ofbiz/trunk/applications/product/src/org/ofbiz/product/product/ProductEvents.java
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/product/src/org/ofbiz/product/product/ProductEvents.java?rev=1541746&r1=1541745&r2=1541746&view=diff
> ==============================================================================
> --- ofbiz/trunk/applications/product/src/org/ofbiz/product/product/ProductEvents.java (original)
> +++ ofbiz/trunk/applications/product/src/org/ofbiz/product/product/ProductEvents.java Wed Nov 13 22:04:33 2013
> @@ -24,6 +24,7 @@ import java.util.Iterator;
>   import java.util.List;
>   import java.util.Map;
>   import java.util.Set;
> +import java.util.regex.Matcher;
>   import java.util.regex.Pattern;
>
>   import javax.servlet.http.HttpServletRequest;
> @@ -1186,8 +1187,11 @@ public class ProductEvents {
>           if (UtilValidate.isNotEmpty(productId) && UtilValidate.isNotEmpty(productTags)) {
>               List<String> matchList = FastList.newInstance();
>               Pattern regex = Pattern.compile("[^\\s\"']+|\"([^\"]*)\"|'([^']*)'");
> -            while (regex.matcher(productTags).find()) {
> -                matchList.add(regex.matcher(productTags).group().replace("'", ""));
> +            Matcher regexMatcher = regex.matcher(productTags);
> +            synchronized (regexMatcher) {
> +                while (regexMatcher.find()) {
> +                    matchList.add(regexMatcher.group().replace("'", ""));
> +                }
>               }
>
>               GenericValue userLogin = null;
>
>