svn commit: r1005467 - /tomcat/trunk/java/org/apache/jasper/servlet/JspServletWrapper.java

[markt]

Author: markt
Date: Thu Oct  7 14:34:29 2010
New Revision: 1005467

URL: http://svn.apache.org/viewvc?rev=1005467&view=rev
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=49986
Thread safety issues in JSP reload process. (timw)

Modified:
    tomcat/trunk/java/org/apache/jasper/servlet/JspServletWrapper.java

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=1005467&r1=1005466&r2=1005467&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/jasper/servlet/JspServletWrapper.java 
(original)
+++ tomcat/trunk/java/org/apache/jasper/servlet/JspServletWrapper.java Thu Oct  
7 14:34:29 2010
@@ -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) {



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