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 <benoitde@sympatico.ca>
> Subject: Re: [PATCH] Re: [PROPOSAL] Modification of the code generated by 
Jasper2
> To: Tomcat Developers List <tomcat-dev@jakarta.apache.org>, Kin-Man Chung 
<Kin-Man.Chung@Eng.Sun.COM>
> 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 
to
> 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 {
      bitmask.set(0);
      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 {
        bitmask.set(1);
        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();
          _jspx_th_eg_foo_0.setBodyContent((javax.servlet.jsp.tagext.BodyContent
) out);
          _jspx_th_eg_foo_0.doInitBody();
        }
        do {
          String           member = (String) pageContext.findAttribute("member")
;
          out.write("\n");
          out.write("<li>");
          out.print( member );
          out.write("</li>\n");
        } while (_jspx_th_eg_foo_0.doAfterBody() == javax.servlet.jsp.tagext.Bod
yTag.EVAL_BODY_AGAIN);
        // } finally {
        bitmask.clear(1);
        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.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
E)

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, 
it
> 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 
executed
> 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
> benoitde@sympatico.ca
> 


--
To unsubscribe, e-mail:   <mailto:tomcat-dev-unsubscribe@jakarta.apache.org>
For additional commands, e-mail: <mailto:tomcat-dev-help@jakarta.apache.org>


Mime
View raw message