Author: markt Date: Wed Oct 3 14:03:51 2018 New Revision: 1842725 URL: http://svn.apache.org/viewvc?rev=1842725&view=rev Log: Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=62757 Correct a regression in the fix for bug 62603 that caused NullPointerExceptions when compiling tag files on first access when development mode was disabled and background compilation was enabled. Based on a patch by Jordi Llach.
Modified: tomcat/trunk/java/org/apache/jasper/compiler/JspRuntimeContext.java tomcat/trunk/java/org/apache/jasper/compiler/TagFileProcessor.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=1842725&r1=1842724&r2=1842725&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/jasper/compiler/JspRuntimeContext.java (original) +++ tomcat/trunk/java/org/apache/jasper/compiler/JspRuntimeContext.java Wed Oct 3 14:03:51 2018 @@ -378,8 +378,7 @@ public final class JspRuntimeContext { for (int i = 0; i < wrappers.length; i++ ) { JspServletWrapper jsw = (JspServletWrapper)wrappers[i]; JspCompilationContext ctxt = jsw.getJspEngineContext(); - // JspServletWrapper also synchronizes on this when - // it detects it has to do a reload + // Sync on JspServletWrapper when calling ctxt.compile() synchronized(jsw) { try { ctxt.compile(); Modified: tomcat/trunk/java/org/apache/jasper/compiler/TagFileProcessor.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/compiler/TagFileProcessor.java?rev=1842725&r1=1842724&r2=1842725&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/jasper/compiler/TagFileProcessor.java (original) +++ tomcat/trunk/java/org/apache/jasper/compiler/TagFileProcessor.java Wed Oct 3 14:03:51 2018 @@ -543,12 +543,11 @@ class TagFileProcessor { wrapper = new JspServletWrapper(ctxt.getServletContext(), ctxt .getOptions(), tagFilePath, tagInfo, ctxt .getRuntimeContext(), tagJar); - rctxt.addWrapper(wrapperUri, wrapper); - // Use same classloader and classpath for compiling tag files wrapper.getJspEngineContext().setClassLoader( ctxt.getClassLoader()); wrapper.getJspEngineContext().setClassPath(ctxt.getClassPath()); + rctxt.addWrapper(wrapperUri, wrapper); } else { // Make sure that JspCompilationContext gets the latest TagInfo // for the tag file. TagInfo instance was created the last 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=1842725&r1=1842724&r2=1842725&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/jasper/servlet/JspServletWrapper.java (original) +++ tomcat/trunk/java/org/apache/jasper/servlet/JspServletWrapper.java Wed Oct 3 14:03:51 2018 @@ -89,13 +89,20 @@ public class JspServletWrapper { private long available = 0L; private final ServletConfig config; private final Options options; - private volatile boolean firstTime = true; - /** Whether the servlet needs reloading on next access */ + /* + * The servlet / tag file needs a compilation check on first access. Use a + * separate flag (rather then theServlet == null / tagHandlerClass == null + * as it avoids the potentially expensive isOutDated() calls in + * ctxt.compile() if there are multiple concurrent requests for the servlet + * / tag before the class has been loaded. + */ + private volatile boolean mustCompile = true; + /* Whether the servlet/tag file needs reloading on next access */ private volatile boolean reload = true; private final boolean isTagFile; private int tripCount; private JasperException compileException; - /** Timestamp of last time servlet resource was modified */ + /* Timestamp of last time servlet resource was modified */ private volatile long servletClassLastModifiedTime; private long lastModificationTest = 0L; private long lastUsageTime = System.currentTimeMillis(); @@ -158,7 +165,7 @@ public class JspServletWrapper { } private boolean getReloadInternal() { - return firstTime || reload && !ctxt.getRuntimeContext().isCompileCheckInProgress(); + return reload && !ctxt.getRuntimeContext().isCompileCheckInProgress(); } public Servlet getServlet() throws ServletException { @@ -171,11 +178,11 @@ public class JspServletWrapper { * 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()) { + if (getReloadInternal() || theServlet == null) { synchronized (this) { // Synchronizing on jsw enables simultaneous loading // of different pages, but not the same page. - if (getReloadInternal()) { + if (getReloadInternal() || theServlet == null) { // This is to maintain the original protocol. destroy(); @@ -193,7 +200,7 @@ public class JspServletWrapper { servlet.init(config); - if (!firstTime) { + if (theServlet != null) { ctxt.getRuntimeContext().incrementJspReloadCount(); } @@ -257,10 +264,12 @@ public class JspServletWrapper { if (ctxt.isRemoved()) { throw new FileNotFoundException(jspUri); } - if (options.getDevelopment() || firstTime ) { + if (options.getDevelopment() || mustCompile) { synchronized (this) { - firstTime = false; - ctxt.compile(); + if (options.getDevelopment() || mustCompile) { + ctxt.compile(); + mustCompile = false; + } } } else { if (compileException != null) { @@ -268,9 +277,14 @@ public class JspServletWrapper { } } - if (getReloadInternal()) { - tagHandlerClass = ctxt.load(); - reload = false; + if (getReloadInternal() || tagHandlerClass == null) { + synchronized (this) { + if (getReloadInternal() || tagHandlerClass == null) { + tagHandlerClass = ctxt.load(); + // Volatile 'reload' forces in order write of 'tagHandlerClass' + reload = false; + } + } } } catch (FileNotFoundException ex) { throw new JasperException(ex); @@ -306,8 +320,12 @@ public class JspServletWrapper { Object target; if (isTagFile) { if (reload) { - tagHandlerClass = ctxt.load(); - reload = false; + synchronized (this) { + if (reload) { + tagHandlerClass = ctxt.load(); + reload = false; + } + } } target = tagHandlerClass.newInstance(); } else { @@ -375,12 +393,13 @@ public class JspServletWrapper { /* * (1) Compile */ - if (options.getDevelopment() || firstTime ) { + if (options.getDevelopment() || mustCompile) { synchronized (this) { - firstTime = false; - - // The following sets reload to true, if necessary - ctxt.compile(); + if (options.getDevelopment() || mustCompile) { + // The following sets reload to true, if necessary + ctxt.compile(); + mustCompile = false; + } } } else { if (compileException != null) { Modified: tomcat/trunk/webapps/docs/changelog.xml URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1842725&r1=1842724&r2=1842725&view=diff ============================================================================== --- tomcat/trunk/webapps/docs/changelog.xml (original) +++ tomcat/trunk/webapps/docs/changelog.xml Wed Oct 3 14:03:51 2018 @@ -126,6 +126,12 @@ utility, <code>JspC</code>, caused by the fix for <bug>53492</bug>, that caused the JSP compiler to hang. (markt) </fix> + <fix> + <bug>62757</bug>: Correct a regression in the fix for <bug>62603</bug> + that caused <code>NullPointerException</code>s when compiling tag files + on first access when development mode was disabled and background + compilation was enabled. Based on a patch by Jordi Llach. (markt) + </fix> </changelog> </subsection> <subsection name="WebSocket"> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org