tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Kin-Man Chung <Kin-Man.Ch...@Eng.Sun.COM>
Subject Re: [PATCH] Re: [PROPOSAL] Modification of the code generated by Jasper2
Date Thu, 09 May 2002 01:18:48 GMT

> Date: Wed, 08 May 2002 20:50:06 -0400 (EDT)
> From: Denis Benoit <>
> Subject: Re: [PATCH] Re: [PROPOSAL] Modification of the code generated by 
> To: Tomcat Developers List <>, Kin-Man Chung 
> MIME-version: 1.0
> Mr Chung,
> On Wed, 8 May 2002, Kin-Man Chung wrote:
> > Denis,
> > 
> > I just had a chance to look at your patch, and I have couple of questions.
> > 
> > In the generated method finallies, there is the code fragment,
> > 
> >     if (bitmask.get(1)) {
> >       if 
(((javax.servlet.jsp.tagext.BodyTag)tags.elementAt(1)).doAfterBody() !=
> >  javax.servlet.jsp.tagext.Tag.EVAL_BODY_INCLUDE)
> >         out = pageContext.popBody();
> >     }
> This code corresponds to the pseudo "finally" that can be found at lines 1068 
> 1092.  It is generated in the generateCustomEnd() method when the tag did have
> a body and implemented the BodyTag interface.
> > 
> > Shouldn't that be doStartTag() instead of doAfterBody()?
> No because it correspond to the "finally" part of a try/finally block of a
> Tag implementing the BodyTag interface.

A fragment for the generated code for SimpleTag in sample is

      // try {
      addTagToVector(tags, 0, _jspx_th_eg_foo_0);
      int _jspx_eval_eg_foo_0 = _jspx_th_eg_foo_0.doStartTag();
      if (_jspx_eval_eg_foo_0 != javax.servlet.jsp.tagext.Tag.SKIP_BODY) {
        // try {
        addTagToVector(tags, 1, _jspx_th_eg_foo_0);
        if (_jspx_eval_eg_foo_0 != javax.servlet.jsp.tagext.Tag.EVAL_BODY_INCLUD
E) {
          out = pageContext.pushBody();
) out);
        do {
          String           member = (String) pageContext.findAttribute("member")
          out.print( member );
        } while (_jspx_th_eg_foo_0.doAfterBody() == javax.servlet.jsp.tagext.Bod
        // } finally {
        addTagToVector(tags, 1, _jspx_th_eg_foo_0);
        if (_jspx_eval_eg_foo_0 != javax.servlet.jsp.tagext.Tag.EVAL_BODY_INCLUD
          out = pageContext.popBody();
        // }

Note the the line

	int _jspx_eval_eg_foo_0 = _jspx_th_eg_foo_0.doStartTag();

and test in the (now commented out) finally block

	if (_jspx_eval_eg_foo_0 != javax.servlet.jsp.tagext.Tag.EVAL_BODY_INCLUD

Clearly, _jspx_eval_eg_foo_0 is the return value of doStartTag, not doAfterBody.

> > 
> > Also, if an exception occurs after the method is invoked, wouldn't it be
> > invoked again in finallies?  I think you need to pass the return value for
> > the method to fanallies instead.
> No, because there is no more try/finally except the "global" one.  The idea
> was to replace the begin of the "try" clause by setting a bit, and when the
> begin of a "finally" clause is started to clear the bit.  So if an exception
> is raised after the "try" and before the "finally", the bit is set, in any
> other condition the bit will be cleared.   You'll notice that whatever would
> be output in the "finally" clause is also added to the "finallies" method,
> that's why if there is an ".doAfterBody()" in a "finally" clause you'll find
> it also in the "finallies" method.

Assume it is doStartTag, if exception happens after doStartTag is called,
say in the body of the Tag, then when the code in finallies is executed,
doStartTag would be called again, which is clearly wrong.  You essentially
call doStartTag twice, once in _jspService, and another in finallies.

> You'll notice that in the generated code, the patch leaves as comments the
> begin of the "try", and the begin of the "finally" and their end.  This way, 
> is easy to validate the logic by following the flow in case an Exception is
> raised somewhere.  We can easily compare the code that would have been 
> with the "try/finally" versus the "finallies" method and the bits.
> Mr Maucherat noticed that the patch do create a BitSet and a Vector, even for
> JSPs that don't have tags, I think it could be avoided if we did some kind of
> lazy initialisation.  My first, dumb, I confess! idea was to put the Vector
> and the BitSet as instance variables, but Mr Barker was quick to point out
> that it would cause thread problem.  My idea was then to have the patch mature
> and if everything was ok to do a second pass to optimize it.
> But we could certainly ask ourselves if this is the right approach to the
> problem.  We know that we can't afford deeply nested try/finally.  The patch
> was tought to disturb as little the control flow of generated page without the
> performance impacts of the nested try/finally.  Another approach would be to
> think if the page could be generated entirely differently with just one
> try/finally and still respect the specs.
> Thanks!
> -- 
> Denis Benoit

To unsubscribe, e-mail:   <>
For additional commands, e-mail: <>

View raw message