Author: cziegeler
Date: Fri Oct  3 08:49:24 2014
New Revision: 1629144

URL: http://svn.apache.org/r1629144
Log:
SLING-3984 : JSP Compilation failure under heavy load. Apply partial patch from 
Viktor Adam

Modified:
    
sling/trunk/bundles/scripting/jsp/src/main/java/org/apache/sling/scripting/jsp/jasper/compiler/JspRuntimeContext.java
    
sling/trunk/bundles/scripting/jsp/src/main/java/org/apache/sling/scripting/jsp/jasper/compiler/TagFileProcessor.java

Modified: 
sling/trunk/bundles/scripting/jsp/src/main/java/org/apache/sling/scripting/jsp/jasper/compiler/JspRuntimeContext.java
URL: 
http://svn.apache.org/viewvc/sling/trunk/bundles/scripting/jsp/src/main/java/org/apache/sling/scripting/jsp/jasper/compiler/JspRuntimeContext.java?rev=1629144&r1=1629143&r2=1629144&view=diff
==============================================================================
--- 
sling/trunk/bundles/scripting/jsp/src/main/java/org/apache/sling/scripting/jsp/jasper/compiler/JspRuntimeContext.java
 (original)
+++ 
sling/trunk/bundles/scripting/jsp/src/main/java/org/apache/sling/scripting/jsp/jasper/compiler/JspRuntimeContext.java
 Fri Oct  3 08:49:24 2014
@@ -31,6 +31,8 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
 
 import javax.servlet.Servlet;
 import javax.servlet.ServletContext;
@@ -229,6 +231,11 @@ public final class JspRuntimeContext {
      */
     private final Map<String, Set<String>> depToJsp = new HashMap<String, 
Set<String>>();
 
+    /**
+     * Locks for loading tag files.
+     */
+    private final ConcurrentHashMap<String, Lock> tagFileLoadingLocks = new 
ConcurrentHashMap<String, Lock>();
+
     // ------------------------------------------------------ Public Methods
 
     public void addJspDependencies(final JspServletWrapper jsw, final 
List<String> deps) {
@@ -313,13 +320,22 @@ public final class JspRuntimeContext {
     }
 
     /**
-     * Remove a  JspServletWrapper.
-     *
-     * @param jspUri JSP URI of JspServletWrapper to remove
-    public void removeWrapper(String jspUri) {
-        jsps.remove(jspUri);
+     * Locks a tag file path. Use this before loading it.
+     * @param tagFilePath Tag file path
+     */
+    public void lockTagFileLoading(final String tagFilePath) {
+        final Lock lock = getTagFileLoadingLock(tagFilePath);
+        lock.lock();
     }
+
+    /**
+     * Unlocks a tag file path. Use this after loading it.
+     * @param tagFilePath Tag file path
      */
+    public void unlockTagFileLoading(final String tagFilePath) {
+        final Lock lock = getTagFileLoadingLock(tagFilePath);
+        lock.unlock();
+    }
 
     /**
      * Process a "destroy" event for this web application context.
@@ -403,5 +419,20 @@ public final class JspRuntimeContext {
         }
     }
 
+    /**
+     * Returns and optionally creates a lock to load a tag file.
+     */
+    private Lock getTagFileLoadingLock(final String tagFilePath) {
+        Lock lock = tagFileLoadingLocks.get(tagFilePath);
+        if (lock == null) {
+            lock = new ReentrantLock();
+            final Lock existingLock = 
tagFileLoadingLocks.putIfAbsent(tagFilePath, lock);
+            if (existingLock != null) {
+                lock = existingLock;
+            }
+        }
+
+        return lock;
+    }
 
 }

Modified: 
sling/trunk/bundles/scripting/jsp/src/main/java/org/apache/sling/scripting/jsp/jasper/compiler/TagFileProcessor.java
URL: 
http://svn.apache.org/viewvc/sling/trunk/bundles/scripting/jsp/src/main/java/org/apache/sling/scripting/jsp/jasper/compiler/TagFileProcessor.java?rev=1629144&r1=1629143&r2=1629144&view=diff
==============================================================================
--- 
sling/trunk/bundles/scripting/jsp/src/main/java/org/apache/sling/scripting/jsp/jasper/compiler/TagFileProcessor.java
 (original)
+++ 
sling/trunk/bundles/scripting/jsp/src/main/java/org/apache/sling/scripting/jsp/jasper/compiler/TagFileProcessor.java
 Fri Oct  3 08:49:24 2014
@@ -533,67 +533,75 @@ class TagFileProcessor {
 
         JspCompilationContext ctxt = compiler.getCompilationContext();
         JspRuntimeContext rctxt = ctxt.getRuntimeContext();
-        JspServletWrapper wrapper = rctxt.getWrapper(tagFilePath);
 
-        if (wrapper == null) {
-            wrapper = new JspServletWrapper(ctxt.getServletContext(), ctxt
-                    .getOptions(), tagFilePath, tagInfo, ctxt
-                    .getRuntimeContext(), compiler.getDefaultIsSession(),
-                     ctxt.getTagFileJarUrl(tagFilePath));
-            wrapper = rctxt.addWrapper(tagFilePath, wrapper);
-
-            // Use same classloader and classpath for compiling tag files
-            
//wrapper.getJspEngineContext().setClassLoader(ctxt.getClassLoader());
-            //wrapper.getJspEngineContext().setClassPath(ctxt.getClassPath());
-        } else {
-            // Make sure that JspCompilationContext gets the latest TagInfo
-            // for the tag file. TagInfo instance was created the last
-            // time the tag file was scanned for directives, and the tag
-            // file may have been modified since then.
-            wrapper.getJspEngineContext().setTagInfo(tagInfo);
-        }
-
-        Class tagClazz;
-        int tripCount = wrapper.incTripCount();
+        rctxt.lockTagFileLoading(tagFilePath);
         try {
-            if (tripCount > 0) {
-                // When tripCount is greater than zero, a circular
-                // dependency exists. The circularily dependant tag
-                // file is compiled in prototype mode, to avoid infinite
-                // recursion.
-
-                JspServletWrapper tempWrapper = new JspServletWrapper(ctxt
-                        .getServletContext(), ctxt.getOptions(),
-                        tagFilePath, tagInfo, ctxt.getRuntimeContext(),
-                        compiler.getDefaultIsSession(),
-                        ctxt.getTagFileJarUrl(tagFilePath));
-                tagClazz = tempWrapper.loadTagFilePrototype();
-                tempVector.add(tempWrapper.getJspEngineContext()
-                        .getCompiler());
+
+            JspServletWrapper wrapper = rctxt.getWrapper(tagFilePath);
+
+            if (wrapper == null) {
+                wrapper = new JspServletWrapper(ctxt.getServletContext(), ctxt
+                      .getOptions(), tagFilePath, tagInfo, ctxt
+                      .getRuntimeContext(), compiler.getDefaultIsSession(),
+                      ctxt.getTagFileJarUrl(tagFilePath));
+                wrapper = rctxt.addWrapper(tagFilePath, wrapper);
+
+                // Use same classloader and classpath for compiling tag files
+                
//wrapper.getJspEngineContext().setClassLoader(ctxt.getClassLoader());
+                
//wrapper.getJspEngineContext().setClassPath(ctxt.getClassPath());
             } else {
-                tagClazz = wrapper.loadTagFile();
+                // Make sure that JspCompilationContext gets the latest TagInfo
+                // for the tag file. TagInfo instance was created the last
+                // time the tag file was scanned for directives, and the tag
+                // file may have been modified since then.
+                wrapper.getJspEngineContext().setTagInfo(tagInfo);
             }
-        } finally {
-            wrapper.decTripCount();
-        }
 
-        // Add the dependants for this tag file to its parent's
-        // dependant list. The only reliable dependency information
-        // can only be obtained from the tag instance.
-        try {
-            Object tagIns = tagClazz.newInstance();
-            if (tagIns instanceof JspSourceDependent) {
-                Iterator iter = ((List) ((JspSourceDependent) tagIns)
-                        .getDependants()).iterator();
-                while (iter.hasNext()) {
-                    parentPageInfo.addDependant((String) iter.next());
+            Class tagClazz;
+            int tripCount = wrapper.incTripCount();
+            try {
+                if (tripCount > 0) {
+                    // When tripCount is greater than zero, a circular
+                    // dependency exists. The circularily dependant tag
+                    // file is compiled in prototype mode, to avoid infinite
+                    // recursion.
+
+                    JspServletWrapper tempWrapper = new JspServletWrapper(ctxt
+                          .getServletContext(), ctxt.getOptions(),
+                          tagFilePath, tagInfo, ctxt.getRuntimeContext(),
+                          compiler.getDefaultIsSession(),
+                          ctxt.getTagFileJarUrl(tagFilePath));
+                    tagClazz = tempWrapper.loadTagFilePrototype();
+                    tempVector.add(tempWrapper.getJspEngineContext()
+                          .getCompiler());
+                } else {
+                    tagClazz = wrapper.loadTagFile();
                 }
+            } finally {
+                wrapper.decTripCount();
+            }
+
+            // Add the dependants for this tag file to its parent's
+            // dependant list. The only reliable dependency information
+            // can only be obtained from the tag instance.
+            try {
+                Object tagIns = tagClazz.newInstance();
+                if (tagIns instanceof JspSourceDependent) {
+                    Iterator iter = ((List) ((JspSourceDependent) tagIns)
+                          .getDependants()).iterator();
+                    while (iter.hasNext()) {
+                        parentPageInfo.addDependant((String) iter.next());
+                    }
+                }
+            } catch (Exception e) {
+                // ignore errors
             }
-        } catch (Exception e) {
-            // ignore errors
-        }
 
-        return tagClazz;
+            return tagClazz;
+
+        } finally {
+            rctxt.unlockTagFileLoading(tagFilePath);
+        }
     }
 
     /*


Reply via email to