You are viewing a plain text version of this content. The canonical link for it is here.
Posted to taglibs-dev@jakarta.apache.org by Kris Schneider <kr...@dotech.com> on 2007/03/27 21:50:43 UTC

Discussion of Bug 33934

http://issues.apache.org/bugzilla/show_bug.cgi?id=33934

After some thought, here's why I think we can actually set the target 
property to null in doEndTag (or doFinally, if we add it) instead of release.

If a request-time value is used (scriptlet expression or EL), then the spec 
requires that the container must reset the property's value between all 
reuses. In other words, setTarget should be called each time the tag is 
(re)used.

If a literal value is used, then the container is not required to reset the 
property's value. First, there's type conversion to deal with. However, 
since the property's type is Object, a new String instance will be used. In 
turn, this will cause doEndTag to throw an exception because String does 
not expose any writable Bean properties.

So, AFAICT, the only way to get a valid value for target is with a 
request-time value.

Feedback?

P.S.
This is in the context of JSP 2.0 and JSTL 1.1. It may still hold true for 
JSP 1.2 and JSTL 1.0, even for different reasons ;-), but I haven't thought 
much about it...

-- 
Kris Schneider <ma...@dotech.com>
D.O.Tech       <http://www.dotech.com/>

---------------------------------------------------------------------
To unsubscribe, e-mail: taglibs-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: taglibs-dev-help@jakarta.apache.org


Re: Discussion of Bug 33934

Posted by Kris Schneider <kr...@dotech.com>.
Kris Schneider wrote:
> Kris Schneider wrote:
> 
>> Henri Yandell wrote:
>>
>>> Not great feedback I'm afraid - the suggestion of fixing things in
>>> doEndTag sounds fine to me (null or ""), but I'm not deep into the
>>> spec. If it's not an attribute of the tag, and it'll get recreated
>>> anew in doStartTag I don't see any reason to avoid GC having a chance.
>>
>>
>>
>> At least it's something ;-). I'd thought about trying to track down 
>> the right place to post some questions to the current JSTL (1.2) 
>> implementors (Glassfish? jstl-spec-public and/or jsp-spec-public on 
>> java.net? or?), but haven't bothered yet. I'll probably just go ahead 
>> with the fix by doing two things:
>>
>> Have org.apache.taglibs.standard.tag.common.core.SetSupport implement 
>> TryCatchFinally. Based on my reading of the JLS, this should maintain 
>> binary compatibility.
>>
>> In the doFinally method, set the target field to "".
> 
> 
> Sigh. That's not really legal either. If it's not a String, then setting 
> it to null might work. The problem is that in between doStartTag and 
> doEndTag/doFinally, user code is allowed to call getTarget and it needs 
> to return the proper value - even if it eventually causes doEndTag to 
> throw an exception. Obviously, I'll cogitate some more...

:-/ Of course, there is no such method as getTarget on either SetSupport or 
SetTag...

>> Just to be clear on what should happen, it won't "get recreated anew 
>> in doStartTag". For a request-time value, the container should always 
>> call setTarget before calling doStartTag/doEndTag. For a literal 
>> value, doEndTag always throws an exception, so there's no need to 
>> worry about persisting the value.
>>
>>> Same for the c:forEach, though I've no great itch to hunt down other 
>>> cases.
>>
>>
>>
>> I haven't looked at forEach yet...
>>
>>> Hen
>>>
>>> On 3/27/07, Kris Schneider <kr...@dotech.com> wrote:
>>>
>>>> On further reflection, perhaps target should be set to "" instead of 
>>>> null
>>>> to maintain exception consistency...
>>>>
>>>> Kris Schneider wrote:
>>>> > http://issues.apache.org/bugzilla/show_bug.cgi?id=33934
>>>> >
>>>> > After some thought, here's why I think we can actually set the target
>>>> > property to null in doEndTag (or doFinally, if we add it) instead of
>>>> > release.
>>>> >
>>>> > If a request-time value is used (scriptlet expression or EL), then 
>>>> the
>>>> > spec requires that the container must reset the property's value 
>>>> between
>>>> > all reuses. In other words, setTarget should be called each time 
>>>> the tag
>>>> > is (re)used.
>>>> >
>>>> > If a literal value is used, then the container is not required to 
>>>> reset
>>>> > the property's value. First, there's type conversion to deal with.
>>>> > However, since the property's type is Object, a new String 
>>>> instance will
>>>> > be used. In turn, this will cause doEndTag to throw an exception 
>>>> because
>>>> > String does not expose any writable Bean properties.
>>>> >
>>>> > So, AFAICT, the only way to get a valid value for target is with a
>>>> > request-time value.
>>>> >
>>>> > Feedback?
>>>> >
>>>> > P.S.
>>>> > This is in the context of JSP 2.0 and JSTL 1.1. It may still hold 
>>>> true
>>>> > for JSP 1.2 and JSTL 1.0, even for different reasons ;-), but I 
>>>> haven't
>>>> > thought much about it...
>>>>
>>>> -- 
>>>> Kris Schneider <ma...@dotech.com>
>>>> D.O.Tech       <http://www.dotech.com/>

-- 
Kris Schneider <ma...@dotech.com>
D.O.Tech       <http://www.dotech.com/>

---------------------------------------------------------------------
To unsubscribe, e-mail: taglibs-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: taglibs-dev-help@jakarta.apache.org


Re: Discussion of Bug 33934

Posted by Kris Schneider <kr...@dotech.com>.
Kris Schneider wrote:
> Henri Yandell wrote:
> 
>> Not great feedback I'm afraid - the suggestion of fixing things in
>> doEndTag sounds fine to me (null or ""), but I'm not deep into the
>> spec. If it's not an attribute of the tag, and it'll get recreated
>> anew in doStartTag I don't see any reason to avoid GC having a chance.
> 
> 
> At least it's something ;-). I'd thought about trying to track down the 
> right place to post some questions to the current JSTL (1.2) 
> implementors (Glassfish? jstl-spec-public and/or jsp-spec-public on 
> java.net? or?), but haven't bothered yet. I'll probably just go ahead 
> with the fix by doing two things:
> 
> Have org.apache.taglibs.standard.tag.common.core.SetSupport implement 
> TryCatchFinally. Based on my reading of the JLS, this should maintain 
> binary compatibility.
> 
> In the doFinally method, set the target field to "".

Sigh. That's not really legal either. If it's not a String, then setting it 
to null might work. The problem is that in between doStartTag and 
doEndTag/doFinally, user code is allowed to call getTarget and it needs to 
return the proper value - even if it eventually causes doEndTag to throw an 
exception. Obviously, I'll cogitate some more...

> Just to be clear on what should happen, it won't "get recreated anew in 
> doStartTag". For a request-time value, the container should always call 
> setTarget before calling doStartTag/doEndTag. For a literal value, 
> doEndTag always throws an exception, so there's no need to worry about 
> persisting the value.
> 
>> Same for the c:forEach, though I've no great itch to hunt down other 
>> cases.
> 
> 
> I haven't looked at forEach yet...
> 
>> Hen
>>
>> On 3/27/07, Kris Schneider <kr...@dotech.com> wrote:
>>
>>> On further reflection, perhaps target should be set to "" instead of 
>>> null
>>> to maintain exception consistency...
>>>
>>> Kris Schneider wrote:
>>> > http://issues.apache.org/bugzilla/show_bug.cgi?id=33934
>>> >
>>> > After some thought, here's why I think we can actually set the target
>>> > property to null in doEndTag (or doFinally, if we add it) instead of
>>> > release.
>>> >
>>> > If a request-time value is used (scriptlet expression or EL), then the
>>> > spec requires that the container must reset the property's value 
>>> between
>>> > all reuses. In other words, setTarget should be called each time 
>>> the tag
>>> > is (re)used.
>>> >
>>> > If a literal value is used, then the container is not required to 
>>> reset
>>> > the property's value. First, there's type conversion to deal with.
>>> > However, since the property's type is Object, a new String instance 
>>> will
>>> > be used. In turn, this will cause doEndTag to throw an exception 
>>> because
>>> > String does not expose any writable Bean properties.
>>> >
>>> > So, AFAICT, the only way to get a valid value for target is with a
>>> > request-time value.
>>> >
>>> > Feedback?
>>> >
>>> > P.S.
>>> > This is in the context of JSP 2.0 and JSTL 1.1. It may still hold true
>>> > for JSP 1.2 and JSTL 1.0, even for different reasons ;-), but I 
>>> haven't
>>> > thought much about it...
>>>
>>> -- 
>>> Kris Schneider <ma...@dotech.com>
>>> D.O.Tech       <http://www.dotech.com/>

-- 
Kris Schneider <ma...@dotech.com>
D.O.Tech       <http://www.dotech.com/>

---------------------------------------------------------------------
To unsubscribe, e-mail: taglibs-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: taglibs-dev-help@jakarta.apache.org


Re: Discussion of Bug 33934

Posted by Henri Yandell <fl...@gmail.com>.
On 3/28/07, Kris Schneider <kr...@dotech.com> wrote:
> Henri Yandell wrote:
> > Not great feedback I'm afraid - the suggestion of fixing things in
> > doEndTag sounds fine to me (null or ""), but I'm not deep into the
> > spec. If it's not an attribute of the tag, and it'll get recreated
> > anew in doStartTag I don't see any reason to avoid GC having a chance.
>
> At least it's something ;-). I'd thought about trying to track down the
> right place to post some questions to the current JSTL (1.2) implementors
> (Glassfish? jstl-spec-public and/or jsp-spec-public on java.net? or?), but
> haven't bothered yet.

I gave up on that one - the mail alias bounces and I get the distinct
feeling that there's no one active on it. So I've been throwing issues
into Glassfish that are spec like and resolving them on our side.

Maybe I just didn't find the right place.

Hen

---------------------------------------------------------------------
To unsubscribe, e-mail: taglibs-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: taglibs-dev-help@jakarta.apache.org


Re: Discussion of Bug 33934

Posted by Kris Schneider <kr...@dotech.com>.
Henri Yandell wrote:
> Not great feedback I'm afraid - the suggestion of fixing things in
> doEndTag sounds fine to me (null or ""), but I'm not deep into the
> spec. If it's not an attribute of the tag, and it'll get recreated
> anew in doStartTag I don't see any reason to avoid GC having a chance.

At least it's something ;-). I'd thought about trying to track down the 
right place to post some questions to the current JSTL (1.2) implementors 
(Glassfish? jstl-spec-public and/or jsp-spec-public on java.net? or?), but 
haven't bothered yet. I'll probably just go ahead with the fix by doing two 
things:

Have org.apache.taglibs.standard.tag.common.core.SetSupport implement 
TryCatchFinally. Based on my reading of the JLS, this should maintain 
binary compatibility.

In the doFinally method, set the target field to "".

Just to be clear on what should happen, it won't "get recreated anew in 
doStartTag". For a request-time value, the container should always call 
setTarget before calling doStartTag/doEndTag. For a literal value, doEndTag 
always throws an exception, so there's no need to worry about persisting 
the value.

> Same for the c:forEach, though I've no great itch to hunt down other cases.

I haven't looked at forEach yet...

> Hen
> 
> On 3/27/07, Kris Schneider <kr...@dotech.com> wrote:
> 
>> On further reflection, perhaps target should be set to "" instead of null
>> to maintain exception consistency...
>>
>> Kris Schneider wrote:
>> > http://issues.apache.org/bugzilla/show_bug.cgi?id=33934
>> >
>> > After some thought, here's why I think we can actually set the target
>> > property to null in doEndTag (or doFinally, if we add it) instead of
>> > release.
>> >
>> > If a request-time value is used (scriptlet expression or EL), then the
>> > spec requires that the container must reset the property's value 
>> between
>> > all reuses. In other words, setTarget should be called each time the 
>> tag
>> > is (re)used.
>> >
>> > If a literal value is used, then the container is not required to reset
>> > the property's value. First, there's type conversion to deal with.
>> > However, since the property's type is Object, a new String instance 
>> will
>> > be used. In turn, this will cause doEndTag to throw an exception 
>> because
>> > String does not expose any writable Bean properties.
>> >
>> > So, AFAICT, the only way to get a valid value for target is with a
>> > request-time value.
>> >
>> > Feedback?
>> >
>> > P.S.
>> > This is in the context of JSP 2.0 and JSTL 1.1. It may still hold true
>> > for JSP 1.2 and JSTL 1.0, even for different reasons ;-), but I haven't
>> > thought much about it...
>>
>> -- 
>> Kris Schneider <ma...@dotech.com>
>> D.O.Tech       <http://www.dotech.com/>

-- 
Kris Schneider <ma...@dotech.com>
D.O.Tech       <http://www.dotech.com/>

---------------------------------------------------------------------
To unsubscribe, e-mail: taglibs-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: taglibs-dev-help@jakarta.apache.org


Re: Discussion of Bug 33934

Posted by Henri Yandell <fl...@gmail.com>.
Not great feedback I'm afraid - the suggestion of fixing things in
doEndTag sounds fine to me (null or ""), but I'm not deep into the
spec. If it's not an attribute of the tag, and it'll get recreated
anew in doStartTag I don't see any reason to avoid GC having a chance.

Same for the c:forEach, though I've no great itch to hunt down other cases.

Hen

On 3/27/07, Kris Schneider <kr...@dotech.com> wrote:
> On further reflection, perhaps target should be set to "" instead of null
> to maintain exception consistency...
>
> Kris Schneider wrote:
> > http://issues.apache.org/bugzilla/show_bug.cgi?id=33934
> >
> > After some thought, here's why I think we can actually set the target
> > property to null in doEndTag (or doFinally, if we add it) instead of
> > release.
> >
> > If a request-time value is used (scriptlet expression or EL), then the
> > spec requires that the container must reset the property's value between
> > all reuses. In other words, setTarget should be called each time the tag
> > is (re)used.
> >
> > If a literal value is used, then the container is not required to reset
> > the property's value. First, there's type conversion to deal with.
> > However, since the property's type is Object, a new String instance will
> > be used. In turn, this will cause doEndTag to throw an exception because
> > String does not expose any writable Bean properties.
> >
> > So, AFAICT, the only way to get a valid value for target is with a
> > request-time value.
> >
> > Feedback?
> >
> > P.S.
> > This is in the context of JSP 2.0 and JSTL 1.1. It may still hold true
> > for JSP 1.2 and JSTL 1.0, even for different reasons ;-), but I haven't
> > thought much about it...
>
> --
> Kris Schneider <ma...@dotech.com>
> D.O.Tech       <http://www.dotech.com/>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: taglibs-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: taglibs-dev-help@jakarta.apache.org
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: taglibs-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: taglibs-dev-help@jakarta.apache.org


Re: Discussion of Bug 33934

Posted by Kris Schneider <kr...@dotech.com>.
On further reflection, perhaps target should be set to "" instead of null 
to maintain exception consistency...

Kris Schneider wrote:
> http://issues.apache.org/bugzilla/show_bug.cgi?id=33934
> 
> After some thought, here's why I think we can actually set the target 
> property to null in doEndTag (or doFinally, if we add it) instead of 
> release.
> 
> If a request-time value is used (scriptlet expression or EL), then the 
> spec requires that the container must reset the property's value between 
> all reuses. In other words, setTarget should be called each time the tag 
> is (re)used.
> 
> If a literal value is used, then the container is not required to reset 
> the property's value. First, there's type conversion to deal with. 
> However, since the property's type is Object, a new String instance will 
> be used. In turn, this will cause doEndTag to throw an exception because 
> String does not expose any writable Bean properties.
> 
> So, AFAICT, the only way to get a valid value for target is with a 
> request-time value.
> 
> Feedback?
> 
> P.S.
> This is in the context of JSP 2.0 and JSTL 1.1. It may still hold true 
> for JSP 1.2 and JSTL 1.0, even for different reasons ;-), but I haven't 
> thought much about it...

-- 
Kris Schneider <ma...@dotech.com>
D.O.Tech       <http://www.dotech.com/>

---------------------------------------------------------------------
To unsubscribe, e-mail: taglibs-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: taglibs-dev-help@jakarta.apache.org