Author: markt
Date: Thu Aug  9 13:42:16 2018
New Revision: 1837726

URL: http://svn.apache.org/viewvc?rev=1837726&view=rev
Log:
Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=62603
Fix a potential race condition when development mode is disabled and background 
compilation checks are enabled. It was possible that some updates would not 
take effect and/or ClassNotFoundExceptions would occur.

Modified:
    tomcat/trunk/java/org/apache/jasper/compiler/JspRuntimeContext.java
    tomcat/trunk/java/org/apache/jasper/servlet/JspServletWrapper.java
    tomcat/trunk/webapps/docs/changelog.xml

Modified: tomcat/trunk/java/org/apache/jasper/compiler/JspRuntimeContext.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/compiler/JspRuntimeContext.java?rev=1837726&r1=1837725&r2=1837726&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/jasper/compiler/JspRuntimeContext.java 
(original)
+++ tomcat/trunk/java/org/apache/jasper/compiler/JspRuntimeContext.java Thu Aug 
 9 13:42:16 2018
@@ -28,11 +28,14 @@ import java.security.CodeSource;
 import java.security.PermissionCollection;
 import java.security.Policy;
 import java.security.cert.Certificate;
+import java.util.ArrayList;
+import java.util.List;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.atomic.AtomicInteger;
 
 import javax.servlet.ServletContext;
+import javax.servlet.ServletException;
 
 import org.apache.jasper.Constants;
 import org.apache.jasper.JspCompilationContext;
@@ -176,6 +179,11 @@ public final class JspRuntimeContext {
      */
     private final Map<String,SmapStratum> smaps = new ConcurrentHashMap<>();
 
+    /**
+     * Flag that indicates if a background compilation check is in progress.
+     */
+    private volatile boolean compileCheckInProgress = false;
+
 
     // ------------------------------------------------------ Public Methods
 
@@ -361,6 +369,11 @@ public final class JspRuntimeContext {
             return;
         }
 
+        List<JspServletWrapper> wrappersToReload = new ArrayList<>();
+        // Tell JspServletWrapper to ignore the reload attribute while this
+        // check is in progress. See BZ 62603.
+        compileCheckInProgress = true;
+
         Object [] wrappers = jsps.values().toArray();
         for (int i = 0; i < wrappers.length; i++ ) {
             JspServletWrapper jsw = (JspServletWrapper)wrappers[i];
@@ -370,6 +383,9 @@ public final class JspRuntimeContext {
             synchronized(jsw) {
                 try {
                     ctxt.compile();
+                    if (jsw.getReload()) {
+                        wrappersToReload.add(jsw);
+                    }
                 } catch (FileNotFoundException ex) {
                     ctxt.incrementRemoved();
                 } catch (Throwable t) {
@@ -380,6 +396,31 @@ public final class JspRuntimeContext {
             }
         }
 
+        // See BZ 62603.
+        // OK to process reload flag now.
+        compileCheckInProgress = false;
+        // Ensure all servlets and tags that need to be reloaded, are reloaded.
+        for (JspServletWrapper jsw : wrappersToReload) {
+            // Triggers reload
+            try {
+                if (jsw.isTagFile()) {
+                    // Although this is a public method, all other paths to 
this
+                    // method use this sync and it is required to prevent race
+                    // conditions during the reload.
+                    synchronized (this) {
+                        jsw.loadTagFile();
+                    }
+                } else {
+                    jsw.getServlet();
+                }
+            } catch (ServletException e) {
+                jsw.getServletContext().log("Servlet reload failed", e);
+            }
+        }
+    }
+
+    public boolean isCompileCheckInProgress() {
+        return compileCheckInProgress;
     }
 
     /**

Modified: tomcat/trunk/java/org/apache/jasper/servlet/JspServletWrapper.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/servlet/JspServletWrapper.java?rev=1837726&r1=1837725&r2=1837726&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/jasper/servlet/JspServletWrapper.java 
(original)
+++ tomcat/trunk/java/org/apache/jasper/servlet/JspServletWrapper.java Thu Aug  
9 13:42:16 2018
@@ -153,15 +153,29 @@ public class JspServletWrapper {
         this.reload = reload;
     }
 
+    public boolean getReload() {
+        return reload;
+    }
+
+    private boolean getReloadInternal() {
+        return firstTime || reload && 
!ctxt.getRuntimeContext().isCompileCheckInProgress();
+    }
+
     public Servlet getServlet() throws ServletException {
-        // DCL on 'reload' requires that 'reload' be volatile
-        // (this also forces a read memory barrier, ensuring the
-        // new servlet object is read consistently)
-        if (reload) {
+        /*
+         * DCL on 'reload' requires that 'reload' be volatile
+         * (this also forces a read memory barrier, ensuring the new servlet
+         * object is read consistently).
+         *
+         * When running in non development mode with a checkInterval it is
+         * possible (see BZ 62603) for a race condition to cause failures
+         * if a Servlet or tag is reloaded while a compile check is running
+         */
+        if (getReloadInternal()) {
             synchronized (this) {
                 // Synchronizing on jsw enables simultaneous loading
                 // of different pages, but not the same page.
-                if (reload) {
+                if (getReloadInternal()) {
                     // This is to maintain the original protocol.
                     destroy();
 
@@ -254,7 +268,7 @@ public class JspServletWrapper {
                 }
             }
 
-            if (reload) {
+            if (getReloadInternal()) {
                 tagHandlerClass = ctxt.load();
                 reload = false;
             }

Modified: tomcat/trunk/webapps/docs/changelog.xml
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1837726&r1=1837725&r2=1837726&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Thu Aug  9 13:42:16 2018
@@ -162,16 +162,22 @@
         Correctly decode URL paths (<code>+</code> should not be decoded to a
         space in the path) in the Jasper class loader. (markt)
       </fix>
+      <fix>
+        <bug>62603</bug>: Fix a potential race condition when development mode
+        is disabled and background compilation checks are enabled. It was
+        possible that some updates would not take effect and/or
+        <code>ClassNotFoundException</code>s would occur. (markt)
+      </fix>
     </changelog>
   </subsection>
-  <subseciton name="WebSocket">
+  <subsection name="WebSocket">
     <changelog>
       <fix>
         <bug>62596</bug>: Remove the limit on the size of the initial HTTP
         upgrade request used to establish the web socket connection. (markt)
       </fix>
     </changelog>
-  </subseciton>
+  </subsection>
   <subsection name="Web applications">
     <changelog>
       <add>



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to