Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=49986

Thread safety issues in JSP reload process. (timw)

git-svn-id: https://svn.apache.org/repos/asf/tomcat/trunk@1005467 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/java/org/apache/jasper/servlet/JspServletWrapper.java b/java/org/apache/jasper/servlet/JspServletWrapper.java
index 2b0d934..2f92446 100644 (file)
--- a/java/org/apache/jasper/servlet/JspServletWrapper.java
+++ b/java/org/apache/jasper/servlet/JspServletWrapper.java
@@ -79,11 +79,13 @@ public class JspServletWrapper {
     private ServletConfig config;
     private Options options;
     private boolean firstTime = true;
-    private boolean reload = true;
+    /** Whether the servlet needs reloading on next access */
+    private volatile boolean reload = true;
     private boolean isTagFile;
     private int tripCount;
     private JasperException compileException;
-    private long servletClassLastModifiedTime;
+    /** Timestamp of last time servlet resource was modified */
+    private volatile long servletClassLastModifiedTime;
     private long lastModificationTest = 0L;
     private Entry<JspServletWrapper> ticket;
 
@@ -131,6 +133,9 @@ public class JspServletWrapper {
     }
 
     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) {
             synchronized (this) {
                 // Synchronizing on jsw enables simultaneous loading
@@ -139,7 +144,7 @@ public class JspServletWrapper {
                     // This is to maintain the original protocol.
                     destroy();
                     
-                    Servlet servlet = null;
+                    final Servlet servlet;
 
                     try {
                         InstanceManager instanceManager = InstanceManagerFactory.getInstanceManager(config);
@@ -160,6 +165,7 @@ public class JspServletWrapper {
 
                     theServlet = servlet;
                     reload = false;
+                    // Volatile 'reload' forces in order write of 'theServlet' and new servlet object
                 }
             }    
         }
@@ -186,6 +192,9 @@ public class JspServletWrapper {
      * @param lastModified Last-modified time of servlet class
      */
     public void setServletClassLastModifiedTime(long lastModified) {
+        // DCL requires servletClassLastModifiedTime be volatile
+        // to force read and write barriers on access/set
+        // (and to get atomic write of long)
         if (this.servletClassLastModifiedTime < lastModified) {
             synchronized (this) {
                 if (this.servletClassLastModifiedTime < lastModified) {