One quick comment - this should be provided by the servlet container.
( i.e. make sure that destroy() is not called while a servlet is in
progress ). Of course, JspServlet is overriding the container and manage
it's own servlets.
I'll look at the code and try to port it to 3.3 - and send more comments
as I go. ( if anyone wants to help - the fix should go in the container,
since in 3.3 jsp-generated servlets are treated as normal servlets ).
( for 3.3 JspServlet - probably a cvs merge will work fine )
Costin
On Wed, 25 Apr 2001, Marc Saegesser wrote:
> 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;
> >
> >
> >
>
