JSP 1.2 tag pooling Bug in utility:for which may affect (many?) other tags...

[Mac Ferguson <mf...@communicopia.com>]

Hi folks,
I'm hoping to open up some discussion on an important subject. I recently
D/L'ed the new beta of Resin servlet engine (2.0.b2), which implements a
custom-tag instance-pooling scheme, and right away I found that some of my
JSPs which use the utility:for tag were behaving unexpectedly. I had a test
page which called utility:for 3 times in succession:

<util:for iterations="3" begin="0" varName="index">
	<%= index%> : <br />
</util:for>
<br />
<util:for iterations="3" begin="0" varName="index">
	<%= index%> : <br />
</util:for>
<br />
<util:for iterations="3" begin="0" varName="index">
	<%= index%> : <br />
</util:for>

which previously looped from 0-2 three times,  produced the following output
under Resin:

0
1
2
3

4

5

on examining the source for the tag and several explanations of the
tag-pooling and spec from the creator of Resin I found the following
problems. The tag initializes its state when it's instance variables are
declared as follows:

public class ForTag extends BodyTagSupport {

    private int iterations;
    private String varName = "_count";
    private int begin = 0;
etc...

Theoretically tags should be stateless, so these default values should be
being assigned in the doStartTag() method not in instance variable
declarations. Another part of the spec which was pointed out to me indicates
that successive calls to the same tag with the same attribute values may not
trigger the setter methods, here's the quote from the spec:

"From the spec,  JSP 10.1, "Once properly set, all properties are expected
to be
persistent, so that if the JSP container ascertains that a property has
already
been set on a given tag handler instance, it needs not set it again.""

which once again implies that if any instance-specific initialization needs
to be done in a tag, it should be done in doStartTag() or some submethod
which will be called on every use of the tag instance.

My first thought was that release() should be implemented to reset state,
but the comments for the lifecycle diagram on page 165 of the spec indicate
that release is "intended to be for relleasing long-term data" and there is
no guarantee that properties are retained or not.

I know that this problem may seem remote and Resin-specific right now, but I
suspect that tag pooling will show up in the next release of almost every
servlet engine, as it is the next logical efficiency which can be optimized
in the servlet/JSP architecture. The behaviour which results from tag
instance pooling should be a consideration in the development and testing of
all of the Jakarta Taglibs tags.

Re: JSP 1.2 tag pooling Bug in utility:for which may affect (many?) other tags...

[Eduardo Pelegri-Llopart <Ed...@Sun.COM>]

JSP 1.1 had conflicting descriptions of tag handler pooling
requirements, and implementations had made conflicting assumptions.  We
locked everybody involved in a closed room without food until white
smoke came up...  You should consult JSP 1.2 for the results.

	- eduard/o


Dennis Sosnoski wrote:
> 
> Mac Ferguson wrote:
> 
> > ...
> > Theoretically tags should be stateless, so these default values should be
> > being assigned in the doStartTag() method not in instance variable
> > declarations. Another part of the spec which was pointed out to me indicates
> > that successive calls to the same tag with the same attribute values may not
> > trigger the setter methods, here's the quote from the spec:
> >
> > "From the spec,  JSP 10.1, "Once properly set, all properties are expected
> > to be
> > persistent, so that if the JSP container ascertains that a property has
> > already
> > been set on a given tag handler instance, it needs not set it again.""
> >
> > which once again implies that if any instance-specific initialization needs
> > to be done in a tag, it should be done in doStartTag() or some submethod
> > which will be called on every use of the tag instance.
> >
> > My first thought was that release() should be implemented to reset state,
> > but the comments for the lifecycle diagram on page 165 of the spec indicate
> > that release is "intended to be for relleasing long-term data" and there is
> > no guarantee that properties are retained or not.
> 
> 5.4.2 of the JSP 1.1 spec says explicitly that "Once all invocations on the tag
> handler are completed, the release method is invoked on it. Once release method
> is invoked all properties are assumed to have been reset to an unspecified
> value."
> 
> Unless something has changed in 1.2 release() is what should be resetting the
> state, including all the property values. That still leaves the issues from
> Shawn's email regarding properties set for one use of the tag carrying over to
> another use of the tag.
> 
> I'd suggest that the spec should either require reset to be called before a tag
> is reused (at least with a different set of attributes), or should have some way
> of explicitly setting an attribute to the unused state. I haven't looked into
> 1.2 to see if it has such a requirement, though.
> 
>   - Dennis

Re: JSP 1.2 tag pooling Bug in utility:for which may affect (many?) other tags...

[Dennis Sosnoski <dm...@sosnoski.com>]

Mac Ferguson wrote:

> ...
> Theoretically tags should be stateless, so these default values should be
> being assigned in the doStartTag() method not in instance variable
> declarations. Another part of the spec which was pointed out to me indicates
> that successive calls to the same tag with the same attribute values may not
> trigger the setter methods, here's the quote from the spec:
>
> "From the spec,  JSP 10.1, "Once properly set, all properties are expected
> to be
> persistent, so that if the JSP container ascertains that a property has
> already
> been set on a given tag handler instance, it needs not set it again.""
>
> which once again implies that if any instance-specific initialization needs
> to be done in a tag, it should be done in doStartTag() or some submethod
> which will be called on every use of the tag instance.
>
> My first thought was that release() should be implemented to reset state,
> but the comments for the lifecycle diagram on page 165 of the spec indicate
> that release is "intended to be for relleasing long-term data" and there is
> no guarantee that properties are retained or not.

5.4.2 of the JSP 1.1 spec says explicitly that "Once all invocations on the tag
handler are completed, the release method is invoked on it. Once release method
is invoked all properties are assumed to have been reset to an unspecified
value."

Unless something has changed in 1.2 release() is what should be resetting the
state, including all the property values. That still leaves the issues from
Shawn's email regarding properties set for one use of the tag carrying over to
another use of the tag.

I'd suggest that the spec should either require reset to be called before a tag
is reused (at least with a different set of attributes), or should have some way
of explicitly setting an attribute to the unused state. I haven't looked into
1.2 to see if it has such a requirement, though.

  - Dennis

Re: JSP 1.2 tag pooling Bug in utility:for which may affect (many?) other tags...

["Craig R. McClanahan" <cr...@apache.org>]

On Fri, 11 May 2001, Eduardo Pelegri-Llopart wrote:

> Mac Ferguson wrote:
> 
> > The behaviour which results from tag instance pooling should be a
> > consideration in the development and testing of all of the Jakarta
> > Taglibs tags.
> 
> 100% agree.
> 

Likewise.  I got bit on some incorrect assumptions when Struts tags
started getting used in containers that pool tag instances as well.

> We spent a fair amount of energy in JSP 1.2 to clarify the semantics of
> tag pooling because it can make a significant difference in some
> applications.

In fact, part of that clarification was some lifecycle diagrams that make
these issues much more understandable.  In the JSP 1.2 PFD2 spec, check
out pages 164, 168, and 178.

> Also, I believe that Tomcat is about to acquire tag
> pooling.

It's work in progress.

> 
> 	- eduard/o 
> 

Craig


> > 
> > Hi folks,
> > I'm hoping to open up some discussion on an important subject. I recently
> > D/L'ed the new beta of Resin servlet engine (2.0.b2), which implements a
> > custom-tag instance-pooling scheme, and right away I found that some of my
> > JSPs which use the utility:for tag were behaving unexpectedly. I had a test
> > page which called utility:for 3 times in succession:
> > 
> > <util:for iterations="3" begin="0" varName="index">
> >         <%= index%> : <br />
> > </util:for>
> > <br />
> > <util:for iterations="3" begin="0" varName="index">
> >         <%= index%> : <br />
> > </util:for>
> > <br />
> > <util:for iterations="3" begin="0" varName="index">
> >         <%= index%> : <br />
> > </util:for>
> > 
> > which previously looped from 0-2 three times,  produced the following output
> > under Resin:
> > 
> > 0
> > 1
> > 2
> > 3
> > 
> > 4
> > 
> > 5
> > 
> > on examining the source for the tag and several explanations of the
> > tag-pooling and spec from the creator of Resin I found the following
> > problems. The tag initializes its state when it's instance variables are
> > declared as follows:
> > 
> > public class ForTag extends BodyTagSupport {
> > 
> >     private int iterations;
> >     private String varName = "_count";
> >     private int begin = 0;
> > etc...
> > 
> > Theoretically tags should be stateless, so these default values should be
> > being assigned in the doStartTag() method not in instance variable
> > declarations. Another part of the spec which was pointed out to me indicates
> > that successive calls to the same tag with the same attribute values may not
> > trigger the setter methods, here's the quote from the spec:
> > 
> > "From the spec,  JSP 10.1, "Once properly set, all properties are expected
> > to be
> > persistent, so that if the JSP container ascertains that a property has
> > already
> > been set on a given tag handler instance, it needs not set it again.""
> > 
> > which once again implies that if any instance-specific initialization needs
> > to be done in a tag, it should be done in doStartTag() or some submethod
> > which will be called on every use of the tag instance.
> > 
> > My first thought was that release() should be implemented to reset state,
> > but the comments for the lifecycle diagram on page 165 of the spec indicate
> > that release is "intended to be for relleasing long-term data" and there is
> > no guarantee that properties are retained or not.
> > 
> > I know that this problem may seem remote and Resin-specific right now, but I
> > suspect that tag pooling will show up in the next release of almost every
> > servlet engine, as it is the next logical efficiency which can be optimized
> > in the servlet/JSP architecture. The behaviour which results from tag
> > instance pooling should be a consideration in the development and testing of
> > all of the Jakarta Taglibs tags.
> 

Re: JSP 1.2 tag pooling Bug in utility:for which may affect (many?) other tags...

[Eduardo Pelegri-Llopart <Ed...@Sun.COM>]

Mac Ferguson wrote:

> The behaviour which results from tag instance pooling should be a
> consideration in the development and testing of all of the Jakarta
> Taglibs tags.

100% agree.

We spent a fair amount of energy in JSP 1.2 to clarify the semantics of
tag pooling because it can make a significant difference in some
applications. Also, I believe that Tomcat is about to acquire tag
pooling.

	- eduard/o 

> 
> Hi folks,
> I'm hoping to open up some discussion on an important subject. I recently
> D/L'ed the new beta of Resin servlet engine (2.0.b2), which implements a
> custom-tag instance-pooling scheme, and right away I found that some of my
> JSPs which use the utility:for tag were behaving unexpectedly. I had a test
> page which called utility:for 3 times in succession:
> 
> <util:for iterations="3" begin="0" varName="index">
>         <%= index%> : <br />
> </util:for>
> <br />
> <util:for iterations="3" begin="0" varName="index">
>         <%= index%> : <br />
> </util:for>
> <br />
> <util:for iterations="3" begin="0" varName="index">
>         <%= index%> : <br />
> </util:for>
> 
> which previously looped from 0-2 three times,  produced the following output
> under Resin:
> 
> 0
> 1
> 2
> 3
> 
> 4
> 
> 5
> 
> on examining the source for the tag and several explanations of the
> tag-pooling and spec from the creator of Resin I found the following
> problems. The tag initializes its state when it's instance variables are
> declared as follows:
> 
> public class ForTag extends BodyTagSupport {
> 
>     private int iterations;
>     private String varName = "_count";
>     private int begin = 0;
> etc...
> 
> Theoretically tags should be stateless, so these default values should be
> being assigned in the doStartTag() method not in instance variable
> declarations. Another part of the spec which was pointed out to me indicates
> that successive calls to the same tag with the same attribute values may not
> trigger the setter methods, here's the quote from the spec:
> 
> "From the spec,  JSP 10.1, "Once properly set, all properties are expected
> to be
> persistent, so that if the JSP container ascertains that a property has
> already
> been set on a given tag handler instance, it needs not set it again.""
> 
> which once again implies that if any instance-specific initialization needs
> to be done in a tag, it should be done in doStartTag() or some submethod
> which will be called on every use of the tag instance.
> 
> My first thought was that release() should be implemented to reset state,
> but the comments for the lifecycle diagram on page 165 of the spec indicate
> that release is "intended to be for relleasing long-term data" and there is
> no guarantee that properties are retained or not.
> 
> I know that this problem may seem remote and Resin-specific right now, but I
> suspect that tag pooling will show up in the next release of almost every
> servlet engine, as it is the next logical efficiency which can be optimized
> in the servlet/JSP architecture. The behaviour which results from tag
> instance pooling should be a consideration in the development and testing of
> all of the Jakarta Taglibs tags.

Re: Tag lifecycle (was: JSP 1.2 tag pooling)

[Eduardo Pelegri-Llopart <Ed...@Sun.COM>]

>         Use 1: <x:foo a="b" c="d"/>
>         Use 2: <x:foo a="b"/>

.....

> but then how is an instance to know that one of its properties needs to be
> "reset"?  That is, what's the state of property 'c' in Use 2 above?
> (Clearly, whatever happens to be "left over" in an instance from an
> arbitrary prior use shouldn't be the default.)

The container cannot use in "Use 2" the tag handler instance it used for
"Use 1".  There is no way to return a value to "default".

In practice this should not have a significant performance impact.

Hope this helps,

	 - eduard/o


Shawn Bayern wrote:
> 
> Mac,
> 
> These are all excellent points.  There are various levels of potential
> errors, including a failure to release() in the first place, or more
> subtly, an incorrect assumption that release() will be called between
> doEndTag() and a subsequent doStartTag().
> 
> The spec makes very clear that properties don't have to be set to the same
> value twice consecutively on the same Tag instance.  A key implementation
> principle that follows from this is that tags should treat their
> properties as logically "read only" -- that is, doStartTag() should copy
> properties if it plans to modify them, since there is no guarantee that
> those properties will be reset before the next doStartTag():
> 
>     a container may implement different pooling stratgies to minimize
>     creation cost, or may hoist setting of properties to reduce cost
>     when a tag handler is inside another iterative tag (p. 161)
> 
> However, I'm not clear what's supposed to happen when a tag instance is
> re-used and the prior invocation needed to specify a property that the
> more recent invocation doesn't specify.  E.g.,
> 
>         Use 1: <x:foo a="b" c="d"/>
>         Use 2: <x:foo a="b"/>
> 
> The spec says pretty clearly clearly that
> 
>     Unspecified attributes/properties should not be set (using a
>     setter method) (p. 161)
> 
> but then how is an instance to know that one of its properties needs to be
> "reset"?  That is, what's the state of property 'c' in Use 2 above?
> (Clearly, whatever happens to be "left over" in an instance from an
> arbitrary prior use shouldn't be the default.)
> 
> Is this clarified in the spec?  (I can't find a clarification, but I have
> have missed it.)
> 
> Shawn
> 
> On Fri, 11 May 2001, Mac Ferguson wrote:
> 
> > Hi folks,
> > I'm hoping to open up some discussion on an important subject. I recently
> > D/L'ed the new beta of Resin servlet engine (2.0.b2), which implements a
> > custom-tag instance-pooling scheme, and right away I found that some of my
> > JSPs which use the utility:for tag were behaving unexpectedly. I had a test
> > page which called utility:for 3 times in succession:
> >
> > <util:for iterations="3" begin="0" varName="index">
> >       <%= index%> : <br />
> > </util:for>
> > <br />
> > <util:for iterations="3" begin="0" varName="index">
> >       <%= index%> : <br />
> > </util:for>
> > <br />
> > <util:for iterations="3" begin="0" varName="index">
> >       <%= index%> : <br />
> > </util:for>
> >
> > which previously looped from 0-2 three times,  produced the following output
> > under Resin:
> >
> > 0
> > 1
> > 2
> > 3
> >
> > 4
> >
> > 5
> >
> > on examining the source for the tag and several explanations of the
> > tag-pooling and spec from the creator of Resin I found the following
> > problems. The tag initializes its state when it's instance variables are
> > declared as follows:
> >
> > public class ForTag extends BodyTagSupport {
> >
> >     private int iterations;
> >     private String varName = "_count";
> >     private int begin = 0;
> > etc...
> >
> > Theoretically tags should be stateless, so these default values should be
> > being assigned in the doStartTag() method not in instance variable
> > declarations. Another part of the spec which was pointed out to me indicates
> > that successive calls to the same tag with the same attribute values may not
> > trigger the setter methods, here's the quote from the spec:
> >
> > "From the spec,  JSP 10.1, "Once properly set, all properties are expected
> > to be
> > persistent, so that if the JSP container ascertains that a property has
> > already
> > been set on a given tag handler instance, it needs not set it again.""
> >
> > which once again implies that if any instance-specific initialization needs
> > to be done in a tag, it should be done in doStartTag() or some submethod
> > which will be called on every use of the tag instance.
> >
> > My first thought was that release() should be implemented to reset state,
> > but the comments for the lifecycle diagram on page 165 of the spec indicate
> > that release is "intended to be for relleasing long-term data" and there is
> > no guarantee that properties are retained or not.
> >
> > I know that this problem may seem remote and Resin-specific right now, but I
> > suspect that tag pooling will show up in the next release of almost every
> > servlet engine, as it is the next logical efficiency which can be optimized
> > in the servlet/JSP architecture. The behaviour which results from tag
> > instance pooling should be a consideration in the development and testing of
> > all of the Jakarta Taglibs tags.

Tag lifecycle (was: JSP 1.2 tag pooling)

[Shawn Bayern <ba...@essentially.net>]

Mac,

These are all excellent points.  There are various levels of potential
errors, including a failure to release() in the first place, or more
subtly, an incorrect assumption that release() will be called between
doEndTag() and a subsequent doStartTag().

The spec makes very clear that properties don't have to be set to the same
value twice consecutively on the same Tag instance.  A key implementation
principle that follows from this is that tags should treat their
properties as logically "read only" -- that is, doStartTag() should copy
properties if it plans to modify them, since there is no guarantee that
those properties will be reset before the next doStartTag():

    a container may implement different pooling stratgies to minimize
    creation cost, or may hoist setting of properties to reduce cost
    when a tag handler is inside another iterative tag (p. 161)

However, I'm not clear what's supposed to happen when a tag instance is
re-used and the prior invocation needed to specify a property that the
more recent invocation doesn't specify.  E.g.,

	Use 1: <x:foo a="b" c="d"/>
	Use 2: <x:foo a="b"/>

The spec says pretty clearly clearly that

    Unspecified attributes/properties should not be set (using a
    setter method) (p. 161)

but then how is an instance to know that one of its properties needs to be
"reset"?  That is, what's the state of property 'c' in Use 2 above?  
(Clearly, whatever happens to be "left over" in an instance from an
arbitrary prior use shouldn't be the default.)

Is this clarified in the spec?  (I can't find a clarification, but I have
have missed it.)

Shawn

On Fri, 11 May 2001, Mac Ferguson wrote:

> Hi folks,
> I'm hoping to open up some discussion on an important subject. I recently
> D/L'ed the new beta of Resin servlet engine (2.0.b2), which implements a
> custom-tag instance-pooling scheme, and right away I found that some of my
> JSPs which use the utility:for tag were behaving unexpectedly. I had a test
> page which called utility:for 3 times in succession:
> 
> <util:for iterations="3" begin="0" varName="index">
> 	<%= index%> : <br />
> </util:for>
> <br />
> <util:for iterations="3" begin="0" varName="index">
> 	<%= index%> : <br />
> </util:for>
> <br />
> <util:for iterations="3" begin="0" varName="index">
> 	<%= index%> : <br />
> </util:for>
> 
> which previously looped from 0-2 three times,  produced the following output
> under Resin:
> 
> 0
> 1
> 2
> 3
> 
> 4
> 
> 5
> 
> on examining the source for the tag and several explanations of the
> tag-pooling and spec from the creator of Resin I found the following
> problems. The tag initializes its state when it's instance variables are
> declared as follows:
> 
> public class ForTag extends BodyTagSupport {
> 
>     private int iterations;
>     private String varName = "_count";
>     private int begin = 0;
> etc...
> 
> Theoretically tags should be stateless, so these default values should be
> being assigned in the doStartTag() method not in instance variable
> declarations. Another part of the spec which was pointed out to me indicates
> that successive calls to the same tag with the same attribute values may not
> trigger the setter methods, here's the quote from the spec:
> 
> "From the spec,  JSP 10.1, "Once properly set, all properties are expected
> to be
> persistent, so that if the JSP container ascertains that a property has
> already
> been set on a given tag handler instance, it needs not set it again.""
> 
> which once again implies that if any instance-specific initialization needs
> to be done in a tag, it should be done in doStartTag() or some submethod
> which will be called on every use of the tag instance.
> 
> My first thought was that release() should be implemented to reset state,
> but the comments for the lifecycle diagram on page 165 of the spec indicate
> that release is "intended to be for relleasing long-term data" and there is
> no guarantee that properties are retained or not.
> 
> I know that this problem may seem remote and Resin-specific right now, but I
> suspect that tag pooling will show up in the next release of almost every
> servlet engine, as it is the next logical efficiency which can be optimized
> in the servlet/JSP architecture. The behaviour which results from tag
> instance pooling should be a consideration in the development and testing of
> all of the Jakarta Taglibs tags.