You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2010/10/07 16:34:29 UTC
svn commit: r1005467 -
/tomcat/trunk/java/org/apache/jasper/servlet/JspServletWrapper.java
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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org