JspServlet, in 3.2.x, takes on some of the responsibilities of a servlet
container in that it controls the lifecycle of the implementation servlet
classes (instantiates them, calls init(), service() and destroy()). We
really should be delegating this stuff to a real servlet container and I
think the Jasper34 refactoring is a going to do that. I just couldn't go
that far in 3.2.2 because we're too close to release to make those kinds of
changes.
> -----Original Message-----
> From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED]]
> Sent: Wednesday, April 25, 2001 7:07 PM
> To: [EMAIL PROTECTED]
> Subject: Re: Tomcat 3.2.2 and Thread synchronization
>
>
>
> 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;
> > >
> > >
> > >
> >
