I'll have also some patches to port back in TC 3.2 mod_jk/ajp13.... - Henri Gomez ___[_]____ EMAIL : [EMAIL PROTECTED] (. .) PGP KEY : 697ECEDD ...oOOo..(_)..oOOo... PGP Fingerprint : 9DF8 1EA8 ED53 2F39 DC9B 904A 364F 80E6 >-----Original Message----- >From: Marc Saegesser [mailto:[EMAIL PROTECTED]] >Sent: Thursday, April 26, 2001 1:40 AM >To: [EMAIL PROTECTED] >Subject: Tomcat 3.2.2 and Thread synchronization > > >I've made some pretty major changes to JspServlet.java to >clean up lots of >thread synchroniztion problems. The commit message summarizes >most of the >changes. Because this is a major change very late in the beta >cycle I would >appreciate it if other developers would give this a through review. Be >kind, my head is still spinning from all this synchronization stuff. > >If no one has any complaints I'll cut a new (and hopfully final) beta >release for Tomcat 3.2.2 in a couple days. > > >> -----Original Message----- >> From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED]] >> Sent: Wednesday, April 25, 2001 6:30 PM >> To: [EMAIL PROTECTED] >> Subject: cvs commit: >jakarta-tomcat/src/share/org/apache/jasper/servlet >> JspServlet.java >> >> >> marcsaeg 01/04/25 16:29:30 >> >> Modified: src/share/org/apache/jasper/resources Tag: tomcat_32 >> messages.properties messages_es.properties >> messages_fr.properties >> src/share/org/apache/jasper/servlet Tag: tomcat_32 >> JspServlet.java >> Log: >> This is a fairly major update. There were numerous thread >> synchronization >> problems involving compiling JSP files. These problems were >> most apparent >> on systems running under load in which page re-compilations might >> happen while pages are being executed and where new requests >> arrive while >> a page compilation is underway. >> >> I've made lots of changes so I'll just summarize the major >differences >> here. >> >> 1) The thread synchronization in doLoadJSP() is quite >different. The >> comments before that method describe the synchronization model >> in detail. >> >> 2) The synchronization in doLoadJSP() is now based on the >> JspServletWrapper object instead of the JspServlet object. >> >> 3) The class loading and the object instantiation used to be >> split between >> doLoadJSP() and the JspServletWrapper. They are now >combined into a >> single synchronization block inside doLoadJSP(). >> >> 4) In JspServletWrapper, the data member theServlet was >used directly by >> several methods. Access to this is now provided only through a >> synchronized accessor method(). Any method that needs to >obtain the >> servlet must call getServlet() and store the result in a >stack variable >> (or other per-thread storage). Multiple threads of >execution through >> JspServletWrapper may be executing different servlet >classes if a page >> compilation happened while other requests were being processed. >> >> 5) When a new JSP implementation class replaces an >existing class, the >> original classes destroy() method should be called so that any >> jspDestroy() method on the page will be executed. However, the >> destroy() >> method can't be invoked until we know that there are no >more requests >> being processed on that instance. To support this, the JSP >> implementation >> class is wrapped inside a new class called JspCountedServlet. >> This class >> provides simple reference counting in the service() method. If the >> classes destroy method is called it flags the class for >destruction but >> does not call the servlet's destroy() method until it >> determines that the >> service method has completed on all threads running in the class. >> >> PR: 1280 >> >> Revision Changes Path >> No revision >> >> >> No revision >> >> >> 1.17.2.9 +3 -1 >> >jakarta-tomcat/src/share/org/apache/jasper/resources/messages.p >roperties >> >> Index: messages.properties >> =================================================================== >> RCS file: >> /home/cvs/jakarta-tomcat/src/share/org/apache/jasper/resources/mes >> sages.properties,v >> retrieving revision 1.17.2.8 >> retrieving revision 1.17.2.9 >> diff -u -r1.17.2.8 -r1.17.2.9 >> --- messages.properties 2001/01/12 04:47:00 1.17.2.8 >> +++ messages.properties 2001/04/25 23:29:28 1.17.2.9 >> @@ -1,4 +1,4 @@ >> -# $Id: messages.properties,v 1.17.2.8 2001/01/12 04:47:00 >larryi Exp $ >> +# $Id: messages.properties,v 1.17.2.9 2001/04/25 23:29:28 >> marcsaeg Exp $ >> # >> # Default localized string information >> # Localized this the Default Locale as is en_US >> @@ -214,3 +214,5 @@ >> jsp.error.unterminated.user.tag=Unterminated user-defined tag: >> ending tag {0} not found or incorrectly nested >> jsp.error.invalid.javaEncoding=Invalid java encodings. Tried >> {0} and then {1}. Both failed. >> jsp.error.needAlternateJavaEncoding=Default java encoding {0} >> is invalid on your java platform. An alternate can be specified >> via the 'javaEncoding' parameter of JspServlet. >> +jsp.error.badcount=Internal error. Attempted to decrement the >> reference count for an unreferenced JSP. >> + >> >> >> >> 1.3.2.7 +3 -1 >> jakarta-tomcat/src/share/org/apache/jasper/resources/messages_es.p >> roperties >> >> Index: messages_es.properties >> =================================================================== >> RCS file: >> /home/cvs/jakarta-tomcat/src/share/org/apache/jasper/resources/mes >> sages_es.properties,v >> retrieving revision 1.3.2.6 >> retrieving revision 1.3.2.7 >> diff -u -r1.3.2.6 -r1.3.2.7 >> --- messages_es.properties 2001/01/12 04:47:00 1.3.2.6 >> +++ messages_es.properties 2001/04/25 23:29:29 1.3.2.7 >> @@ -1,4 +1,4 @@ >> -# $Id: messages_es.properties,v 1.3.2.6 2001/01/12 04:47:00 >> larryi Exp $ >> +# $Id: messages_es.properties,v 1.3.2.7 2001/04/25 23:29:29 >> marcsaeg Exp $ >> # >> # Default localized string information >> # Localized para Locale es_ES >> @@ -204,3 +204,5 @@ >> jspc.error.emptyWebApp=-webapp necesita un argumento de archivo >> jsp.error.no.more.content=Final del contenido alcanzado >> mientras se requeria mas entrada: anidamiento de tag s erroneo o >> tag no terminado >> jsp.error.unterminated.user.tag=Tag definida por el usuario no >> terminada: tag de finalizacion {0} no encontrado o >incorrectamente anidado >> +jsp.error.badcount=Internal error. Attempted to decrement the >> reference count for an unreferenced JSP. >> + >> >> >> >> 1.1.2.3 +3 -1 >> jakarta-tomcat/src/share/org/apache/jasper/resources/messages_fr.p >> roperties >> >> Index: messages_fr.properties >> =================================================================== >> RCS file: >> /home/cvs/jakarta-tomcat/src/share/org/apache/jasper/resources/mes >> sages_fr.properties,v >> retrieving revision 1.1.2.2 >> retrieving revision 1.1.2.3 >> diff -u -r1.1.2.2 -r1.1.2.3 >> --- messages_fr.properties 2001/01/12 04:47:00 1.1.2.2 >> +++ messages_fr.properties 2001/04/25 23:29:29 1.1.2.3 >> @@ -1,4 +1,4 @@ >> -# $Id: messages_fr.properties,v 1.1.2.2 2001/01/12 04:47:00 >> larryi Exp $ >> +# $Id: messages_fr.properties,v 1.1.2.3 2001/04/25 23:29:29 >> marcsaeg Exp $ >> # >> # Default localized string information >> # Localized this the Default Locale as is fr_FR >> @@ -208,3 +208,5 @@ >> jspc.error.emptyWebApp=-webapp nécessite un argument suivant >> de type fichier >> jsp.error.no.more.content=Fin de contenu atteinte alors que >> plus d''analyse est nécessaire: Un tag non terminé ou une erreur >> de tag imbriqué? >> jsp.error.unterminated.user.tag=Tag utilisateur non terminé: >> Le tag de fermeture {0} est introuvable ou incorrectement imbriqué >> +jsp.error.badcount=Internal error. Attempted to decrement the >> reference count for an unreferenced JSP. >> + >> >> >> >> No revision >> >> >> No revision >> >> >> 1.3.2.6 +181 -51 >> jakarta-tomcat/src/share/org/apache/jasper/servlet/JspServlet.java >> >> Index: JspServlet.java >> =================================================================== >> RCS file: >> /home/cvs/jakarta-tomcat/src/share/org/apache/jasper/servlet/JspSe >> rvlet.java,v >> retrieving revision 1.3.2.5 >> retrieving revision 1.3.2.6 >> diff -u -r1.3.2.5 -r1.3.2.6 >> --- JspServlet.java 2001/04/25 21:58:41 1.3.2.5 >> +++ JspServlet.java 2001/04/25 23:29:30 1.3.2.6 >> @@ -91,14 +91,97 @@ >> * >> * @author Anil K. Vijendran >> * @author Harish Prabandham >> + * @author Marc A. Saegesser >> */ >> public class JspServlet extends HttpServlet { >> >> + /** >> + * Adds reference counting to the JSP implementation >servlet. This >> + * is required to handle the case where a JSP >> implementation servlet >> + * is executing requests on several threads when a new >> implementation >> + * arrives (the JSP source was updated). We need to >wait until all >> + * the requests complete before calling the implementation >> servlet's >> + * destroy() method. >> + */ >> + class JspCountedServlet extends HttpServlet >> + { >> + private Servlet servlet = null; >> + private int threadCount = 0; >> + private boolean destroyed = false; >> + >> + public JspCountedServlet(Servlet servlet) >> + { >> + this.servlet = servlet; >> + } >> + >> + public void init(ServletConfig config) throws >> ServletException, JasperException >> + { >> + try{ >> + servlet.init(config); >> + }catch(NullPointerException e){ >> + throw new JasperException(e); >> + } >> + } >> + >> + public void service(HttpServletRequest req, >> HttpServletResponse res) throws ServletException, IOException, >> JasperException >> + { >> + try{ >> + incrementCount(); >> + servlet.service(req, res); >> + }catch(NullPointerException e){ >> + throw new JasperException(e); >> + }finally{ >> + decrementCount(); >> + } >> + } >> + >> + /* >> + * Flags this servlet for destrction once all active >> requests have completed. >> + * After calling this method it is invalid to >call service(). >> + */ >> + public void destroy() >> + { >> + destroyed = true; >> + if(getCount() == 0) >> + doDestroy(); >> + } >> + >> + private void doDestroy() >> + { >> + try{ >> + servlet.destroy(); >> + servlet = null; >> + }catch(NullPointerException e){ >> + } >> + } >> + >> + private synchronized void incrementCount() >> + { >> + threadCount++; >> + } >> + >> + private synchronized void decrementCount() >> + { >> + if(threadCount <= 0){ >> + Constants.message("jsp.error.badcount", >Logger.ERROR); >> + return; >> + } >> + >> + --threadCount; >> + if(threadCount == 0 && destroyed) >> + doDestroy(); >> + } >> + >> + private synchronized int getCount() >> + { >> + return threadCount; >> + } >> + } >> + >> class JspServletWrapper { >> - Servlet theServlet; >> + JspCountedServlet theServlet; >> String jspUri; >> boolean isErrorPage; >> - // ServletWrapper will set this >> Class servletClass; >> >> JspServletWrapper(String jspUri, boolean isErrorPage) { >> @@ -107,23 +190,46 @@ >> this.theServlet = null; >> } >> >> - private void load() throws JasperException, >ServletException { >> + public synchronized void instantiateServlet(Class >> servletClass) throws JasperException, ServletException >> + { >> try { >> - // Class servletClass = (Class) >loadedJSPs.get(jspUri); >> - // This is to maintain the original protocol. >> - destroy(); >> + this.servletClass = servletClass; >> + >> + // If we're replacing an existing JSP >> Implementation class, then >> + // schedule it for destruction >> + if(theServlet != null) >> + theServlet.destroy(); >> + >> + // Create an instance of the JSP >implementation class >> + Servlet servlet = (Servlet) >servletClass.newInstance(); >> + // Set the class loader >> + if(servlet instanceof HttpJspBase) { >> + >> >((HttpJspBase)servlet).setClassLoader(JspServlet.this.parentCla >ssLoader); >> + } >> + >> + // Wrap this servlet in a counted servlet >> + theServlet = new JspCountedServlet(servlet); >> + >> + // Call the JSP Implementation servlet's >> init() method. This >> + // will cause the page's jspInit() method to >> be invoked if one exists. >> + theServlet.init(JspServlet.this.config); >> >> - theServlet = (Servlet) servletClass.newInstance(); >> } catch(Exception ex) { >> throw new JasperException(ex); >> } >> - theServlet.init(JspServlet.this.config); >> - if(theServlet instanceof HttpJspBase) { >> - HttpJspBase h = (HttpJspBase) theServlet; >> - >h.setClassLoader(JspServlet.this.parentClassLoader); >> - } >> + >> } >> >> + public synchronized Servlet getServlet() >> + { >> + return theServlet; >> + } >> + >> + public synchronized boolean isInstantiated() >> + { >> + return theServlet != null; >> + } >> + >> private void loadIfNecessary(HttpServletRequest req, >> HttpServletResponse res) >> throws JasperException, ServletException, >> FileNotFoundException >> { >> @@ -149,10 +255,7 @@ >> }, >> Logger.INFORMATION); >> >> - if(loadJSP(jspUri, cp, isErrorPage, req, res) >> - || theServlet == null) { >> - load(); >> - } >> + loadJSP(jspUri, cp, isErrorPage, req, res); >> } >> >> public void service(HttpServletRequest request, >> @@ -160,21 +263,23 @@ >> boolean precompile) >> throws ServletException, IOException, >FileNotFoundException >> { >> + Servlet servlet = null; >> try { >> loadIfNecessary(request, response); >> + servlet = getServlet(); >> >> // If a page is to only to be precompiled return. >> if(precompile) >> return; >> >> - if(theServlet instanceof SingleThreadModel) { >> + if(servlet instanceof SingleThreadModel) { >> // sync on the wrapper so that the freshness >> // of the page is determined right >before servicing >> synchronized (this) { >> - theServlet.service(request, response); >> + servlet.service(request, response); >> } >> } else { >> - theServlet.service(request, response); >> + servlet.service(request, response); >> } >> >> } catch(FileNotFoundException ex) { >> @@ -216,7 +321,8 @@ >> } >> } >> >> - public void destroy() { >> + public void destroy() >> + { >> if(theServlet != null) >> theServlet.destroy(); >> } >> @@ -272,7 +378,6 @@ >> "<none>" >> }, Logger.DEBUG); >> } >> - // System.out.println("JspServlet: init " + >> config.getServletName() ); >> if( loader==null ) { >> if( jdk12 ) { >> try { >> @@ -307,11 +412,18 @@ >> throws ServletException, IOException >> { >> boolean isErrorPage = exception != null; >> + JspServletWrapper wrapper = null; >> >> - JspServletWrapper wrapper = (JspServletWrapper) >> jsps.get(jspUri); >> - if(wrapper == null) { >> - wrapper = new JspServletWrapper(jspUri, isErrorPage); >> - jsps.put(jspUri, wrapper); >> + /* >> + * Several threads may be handling requests for the >> same jspUri. >> + * Only one of them is allowed to create the >JspServletWrapper. >> + */ >> + synchronized(this){ >> + wrapper = (JspServletWrapper) jsps.get(jspUri); >> + if(wrapper == null) { >> + wrapper = new JspServletWrapper(jspUri, >isErrorPage); >> + jsps.put(jspUri, wrapper); >> + } >> } >> >> wrapper.service(request, response, precompile); >> @@ -437,6 +549,36 @@ >> * @param classpath explicitly set the JSP compilation path. >> * @return true if JSP files is newer >> */ >> + >> + /* >> + * A word about the thread synchronization below. The call to >> + * compiler.isOutDated() is outside the synchronization >> block on purpose. >> + * The expectation is that for the vast majority of cases >> the JSP source file >> + * will not have changed and there will be no need to >> recompile the >> + * implementation class. For those cases when a compile >> is required, we >> + * enter a block that is synchronized on the >> JspServletWrapper object for >> + * this JSP page. Because the initial out dated check is >> unsynchronized, it >> + * is possible for more than one request to attempt to >> enter the synchronized >> + * compile block. The compile() method contains performs >> an outdated check >> + * of its own. The first thread into the block will cause >> a compile, the >> + * subsequent threads will essentially skip the >> compilation and instantiation >> + * steps. >> + * >> + * One other thing to note is that there is a window of >> time between the >> + * compiler.compile() call and the end of the synchronized >> block where a new >> + * thread entering doLoadJSP() will be told that that >> implementation class is >> + * up to date even though the code in the synchronized >> block has not >> + * completed loading and instantiating the class. In this >> case doLoadJSP() >> + * will return false without attempting to compile the >> class. This is OK >> + * because the JspServletWrapper.getServlet() method >used by the >> + * JspServletWrapper.service() method is synchronized. >> Thus, the service >> + * method will not receive the servlet class until it has >> been completely loaded and >> + * instantiated. >> + * >> + * The bottom line is that we avoid synchronizing a fairly >> expensive >> + * operation (isOutDated) but pay a small price of some >> unnecessary >> + * compilation attempts in the atypical case of a modified >> JSP file. >> + */ >> protected boolean doLoadJSP(String jspUri, String classpath, >> boolean isErrorPage, >> HttpServletRequest req, HttpServletResponse res) >> throws JasperException, FileNotFoundException >> @@ -445,25 +587,26 @@ >> if( jsw==null ) { >> throw new JasperException("Can't happen - >> JspServletWrapper=null"); >> } >> - // Class jspClass = (Class) loadedJSPs.get(jspUri); >> - boolean firstTime = jsw.servletClass == null; >> JspCompilationContext ctxt = new >> JspEngineContext(loader, classpath, >> >> context, jspUri, >> >> isErrorPage, options, >> >req, res); >> boolean outDated = false; >> >> - Compiler compiler = null; >> - synchronized(this){ >> - compiler = ctxt.createCompiler(); >> - } >> + Compiler compiler = ctxt.createCompiler(); >> >> try { >> + >> + >> outDated = compiler.isOutDated(); >> - if( (jsw.servletClass == null) || outDated ) { >> - synchronized ( this ) { >> - if((jsw.servletClass == null) || >> (compiler.isOutDated() )) { >> - outDated = compiler.compile(); >> + if(!jsw.isInstantiated() || outDated ) { >> + synchronized(jsw){ >> + outDated = compiler.compile(); >> + if(!jsw.isInstantiated() || outDated) { >> + if( null ==ctxt.getServletClassName() ) { >> + compiler.computeServletClassName(); >> + } >> + >> jsw.instantiateServlet(loader.loadClass(ctxt.getFullClassName())); >> } >> } >> } >> @@ -472,25 +615,12 @@ >> throw ex; >> } catch(JasperException ex) { >> throw ex; >> + } catch(ClassNotFoundException cex) { >> + throw new >> JasperException(Constants.getString("jsp.error.unable.load"), >> + cex); >> } catch(Exception ex) { >> throw new >> JasperException(Constants.getString("jsp.error.unable.compile"), >> ex); >> - } >> - >> - // Reload only if it's outdated >> - if((jsw.servletClass == null) || outDated) { >> - try { >> - if( null ==ctxt.getServletClassName() ) { >> - compiler.computeServletClassName(); >> - } >> - jsw.servletClass = >> loader.loadClass(ctxt.getFullClassName()); >> - //loadClass(ctxt.getFullClassName(), true); >> - } catch(ClassNotFoundException cex) { >> - throw new >> JasperException(Constants.getString("jsp.error.unable.load"), >> - cex); >> - } >> - >> - // loadedJSPs.put(jspUri, jspClass); >> } >> >> return outDated; >> >> >> >
