You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ofbiz.apache.org by Adrian Crum <ad...@sandglass-software.com> on 2013/11/14 13:03:56 UTC
Re: svn commit: r1541746 - /ofbiz/trunk/applications/product/src/org/ofbiz/product/product/ProductEvents.java
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/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;