Casey, I haven't looked at the details but the concept looks really
great! I'm looking forward to a corresponding implementation for 4.0.
A couple of interspersed comments below:
On Sun, 18 Mar 2001, Casey Lucas wrote:
>
> Ok, here's the patch. The attached files apply to tc 3.3. They
> allow tag handler pooling per the jsp spec. The files come with
> a few questions and comments.
>
> Patch info:
> ----------
> TagPoolManagerInterceptor.java should be placed in
> src/facade22/org/apache/tomcat/facade
>
> TagPoolManager.java
> TagPoolManagerImpl.java
> TagHandlerPool.java
> TagHandlerPoolImpl.java
> should be placed in src/share/org/apache/jasper/runtime
>
> TagPoolManagerGenerator.java
> TagPoolGenerator.java
> should be placed in src/share/org/apache/jasper/runtime
>
> The others are diffs.
>
>
> Questions:
> ---------
> 1. Each web application wide TagPoolManager is stored in application scope
> as an attribute. Of course this means that an application could change it.
> Is this acceptable?
>
We're doing that kind of thing in 4.0 today with the classpath and class
loader being passed from Catalina. The good news: it works. The bad
news: it exposes information to change that should not really be
changeable.
The other alternative would be some sort of global static variable that
Jasper knows about -- at least in 4.0 each webapp's Jasper is loaded by a
separate class loader, so statics would be unique.
> 2. Tag handlers are ALWAYS returned to the tag pool. Even in the case of
> exceptions, tag handlers will be reused. Should tags be removed from the
> pool if an exception is thrown during the normal set of tag calls (doStartTag,
> doInitBody, etc.) ?
>
I believe we should plan on reuse, even if exceptions are thrown by this
tag itself.
For example, in Struts I have tags that check for valid combinations of
attributes being set in the doStartTag() method (you need either "x" or
"y"; if you use "a" you must also use "b"; that sort of thing) and throw
an exception if the condition is violated. There's nothing wrong with the
tag instance itself - it was just used wrong in the page.
>
> Caveats:
> -------
> 1. Tag reuse has some important implications for the tag developer.
> In general, the handler must be repeatedly usable without calls
> to Tag.release. For example, handlers should be sure to reset
> certain variables to a well known state in doEndTag. Only those
> variables that change during the processing of the tag need to be
> reset. Those set via attribute setters (and not changed) are ok.
>
Even if you are running under JSP 1.1 (i.e. Tomcat 3.x), tag developers
will find it useful to review the JSP 1.2 Proposed Final Draft spec. It
includes lifecycle diagrams on a tag that identify precisely when the
container will call which methods.
> Note that some of the tags in the watchdog tests do not support
> reuse. So don't enable pooling for watchdog tests.
>
Better would be to fix the tests so they work correctly. At least on the
Watchdog 4.0 tests, I can probably acquire some resources to help us get
these right.
> 2. Because handlers are always returned to the pool, exceptions
> may cause handlers to miss doEndTag and therefore possibly miss
> cleanup / state reset. The new TryCatchFinally interface fixes
> this problem, but it's not available in jsp 1.1.
>
How about a policy where you don't recycle any tag instances *after* the
one that threw the exception? Would that cover all the possible cases?
>
> Comments would be appreciated (and would help my motivation toward
> a 4.x patch :) )
>
Yes please :-)
> -Casey
Craig McClanahan
