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.properties
>
> 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.parentClassLoader);
> + }
> +
> + // 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;
>
>
>
