Author: jacopoc
Date: Fri Apr 20 12:53:35 2012
New Revision: 1328356

URL: http://svn.apache.org/viewvc?rev=1328356&view=rev
Log:
The cache of parsed Groovy scripts was not thread safe; this issue, in 
instances with several concurrent threads running the same script the first 
time (i.e. not cached) could cause the same script parsed multiple times and 
then added to the cache (overriding the previous value); this event was causing 
the clearing of caches in Freemarker; because of a bug in Freemarker [*] this 
could cause a deadlock.
 The issue is present on all versions of Freemarker but it is less frequent on 
latest version because of the refactoring of caches happened after release 
2.3.10.

[*] 
https://sourceforge.net/tracker/?func=detail&aid=3519805&group_id=794&atid=100794

Modified:
    ofbiz/trunk/framework/base/src/org/ofbiz/base/util/GroovyUtil.java

Modified: ofbiz/trunk/framework/base/src/org/ofbiz/base/util/GroovyUtil.java
URL: 
http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/util/GroovyUtil.java?rev=1328356&r1=1328355&r2=1328356&view=diff
==============================================================================
--- ofbiz/trunk/framework/base/src/org/ofbiz/base/util/GroovyUtil.java 
(original)
+++ ofbiz/trunk/framework/base/src/org/ofbiz/base/util/GroovyUtil.java Fri Apr 
20 12:53:35 2012
@@ -127,10 +127,17 @@ public class GroovyUtil {
                 } else {
                     scriptClass = parseClass(scriptUrl.openStream(), location);
                 }
-                if (Debug.verboseOn()) {
-                    Debug.logVerbose("Caching Groovy script at: " + location, 
module);
+                synchronized (parsedScripts) {
+                    Class<?> scriptClassCached = parsedScripts.get(location);
+                    if (scriptClassCached == null) {
+                        if (Debug.verboseOn()) {
+                            Debug.logVerbose("Caching Groovy script at: " + 
location, module);
+                        }
+                        parsedScripts.put(location, scriptClass);
+                    } else {
+                        scriptClass = scriptClassCached;
+                    }
                 }
-                parsedScripts.put(location, scriptClass);
             }
             return scriptClass;
         } catch (Exception e) {
@@ -177,10 +184,18 @@ public class GroovyUtil {
             Class<?> scriptClass = parsedScripts.get(script);
             if (scriptClass == null) {
                 scriptClass = loadClass(script);
-                if (Debug.verboseOn()) Debug.logVerbose("Caching Groovy 
script: " + script, module);
-                parsedScripts.put(script, scriptClass);
+                synchronized (parsedScripts) {
+                    Class<?> scriptClassCached = parsedScripts.get(script);
+                    if (scriptClassCached == null) {
+                        if (Debug.verboseOn()) {
+                            Debug.logVerbose("Caching Groovy script: " + 
script, module);
+                        }
+                        parsedScripts.put(script, scriptClass);
+                    } else {
+                        scriptClass = scriptClassCached;
+                    }
+                }
             }
-
             return InvokerHelper.createScript(scriptClass, 
getBinding(context)).run();
         } catch (CompilationFailedException e) {
             String errMsg = "Error loading Groovy script [" + script + "]: " + 
e.toString();


Reply via email to