You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by Hans Bergsten <ha...@gefionsoftware.com> on 2000/07/03 05:56:36 UTC
Incorrect code for custom actions [Was: Re: 3.2 beta status update]
Hans Bergsten wrote:
> [...]
> I've tested the 3.2 beta with two tag libraries that were working fine with
> Tomcat 3.1, using JDK 1.3 and JDK 1.2.2 on Windows NT. I noticed the
> following problems:
> [...]
> 3) Incorrect code generated for some custom actions.
Okay, I believe I have tracked down this problem now. But since the code for
generating the JSP implementation class is far from obvious, I would appreciate
if someone who's familiar with the code can confirm my findings.
The problem occurs in this scenario. First, the generation of the JSP
implementation
class for one page fails, due to a missing setter method for a custom action
attribute. This results in a CompileException being thrown by the
TagBeginGenerator
generateSetters() method. This is the expected behavior.
After this failure, though, processing of *any* page containing a custom action
results in invalid code being generated. Example:
com.ora.jsp.tags.counter.IncrementCounterTag _jspx_th_ora_incrementCounter_0 =
new com.ora.jsp.tags.counter.IncrementCounterTag();
_jspx_th_ora_incrementCounter_0.setPageContext(pageContext);
_jspx_th_ora_incrementCounter_0.setParent(_jspx_th_iob_dbStore_0);
The _jspx_th_iob_dbStore_0 variable refers to the tag handler in the first page,
i.e. the one with the missing setter method; it's not defined at all in this
page.
The reason is that the state of a static variable in TagGeneratorBase is busted:
static private Stack tagHandlerStack = new Stack();
This Stack seems to be intended to hold the nesting hierarchy for action
elements within a page. I have not traced through all code, but it seems like
if the code generation goes well, the state of the Stack is maintained by an
equal number of push() and pop() calls, but if the code generation fails, junk
is left on the Stack.
I can't understand why this is a static variable; making it a regular instance
variable should solve the problem since a new instance of TagGeneratorBase is
created for each page. Having it as a static probably means that there are
multi-threading issues as well, even though I haven't run into them yet.
There's one more static in this class:
static private Hashtable tagVarNumbers = new Hashtable();
I'm not sure how it's used, but it can probably cause similar problems.
My suggestion is to simple make both these variables regular instance variables.
If I'm missing something, please let me know.
Hans
--
Hans Bergsten hans@gefionsoftware.com
Gefion Software http://www.gefionsoftware.com
Re: Incorrect code for custom actions [Was: Re: 3.2 beta status update]
Posted by Hans Bergsten <ha...@gefionsoftware.com>.
Danno Ferrin wrote:
>
> Here is a quick fix that should solve the problem in a single threaded
> environment. And you are right on about the race conditions, where multiple
> paralell compilations could cause a race, but there is a lock in JspServlet
> where it locks on "this" that solves most races, but in the case where a
> pool of servlet instances services the JspServlet there could be a
> colission. IIRC such pooling is not barred by the spec, (and there are
> other ways to induce that race) so synching on TagGeneratorBase.class in
> lines 178-183 of .../jasper/compiler/Compiler.java should end that race, it
> appears to me to be the best gate point around the JspParseEventListener
> class, which is the only class to use instances of TagGeneratorBase
> sub-classes and classes. A better long-term solution would be to move the
> services provided by those static classes into the JspCompliationContext
> class (which would require an API change which instantly makes it a 3.3
> target)
>
> But I digress, here is a patch you can try.
> [...]
Thanks for your help. I had already started to fix it in a different way
when I got your mail, see the commit message. If you see any problems
with my solution, please let me know.
Hans
--
Hans Bergsten hans@gefionsoftware.com
Gefion Software http://www.gefionsoftware.com
Re: Incorrect code for custom actions [Was: Re: 3.2 beta status update]
Posted by Danno Ferrin <sh...@earthlink.net>.
Here is a quick fix that should solve the problem in a single threaded
environment. And you are right on about the race conditions, where multiple
paralell compilations could cause a race, but there is a lock in JspServlet
where it locks on "this" that solves most races, but in the case where a
pool of servlet instances services the JspServlet there could be a
colission. IIRC such pooling is not barred by the spec, (and there are
other ways to induce that race) so synching on TagGeneratorBase.class in
lines 178-183 of .../jasper/compiler/Compiler.java should end that race, it
appears to me to be the best gate point around the JspParseEventListener
class, which is the only class to use instances of TagGeneratorBase
sub-classes and classes. A better long-term solution would be to move the
services provided by those static classes into the JspCompliationContext
class (which would require an API change which instantly makes it a 3.3
target)
But I digress, here is a patch you can try.
Index: TagBeginGenerator.java
===================================================================
RCS file:
/home/cvs/jakarta-tomcat/src/share/org/apache/jasper/compiler/TagBegin
Generator.java,v
retrieving revision 1.14
diff -U3 -r1.14 TagBeginGenerator.java
--- TagBeginGenerator.java 2000/06/14 22:51:52 1.14
+++ TagBeginGenerator.java 2000/07/03 05:46:16
@@ -199,11 +199,13 @@
String attrName = attributes[i].getName();
Method m = tc.getSetterMethod(attrName);
- if (m == null)
+ if (m == null) {
+ tagEnd();
throw new CompileException
(start, Constants.getString
("jsp.error.unable.to_find_method",
new Object[] { attrName }));
+ }
writer.println(thVarName+"."+m.getName()+"("+attrValue+");");
}
----- Original Message -----
From: "Hans Bergsten" <ha...@gefionsoftware.com>
To: <to...@jakarta.apache.org>
Sent: Sunday, July 02, 2000 9:56 PM
Subject: Incorrect code for custom actions [Was: Re: 3.2 beta status update]
> Hans Bergsten wrote:
> > [...]
> > I've tested the 3.2 beta with two tag libraries that were working fine
with
> > Tomcat 3.1, using JDK 1.3 and JDK 1.2.2 on Windows NT. I noticed the
> > following problems:
> > [...]
> > 3) Incorrect code generated for some custom actions.
>
> Okay, I believe I have tracked down this problem now. But since the code
for
> generating the JSP implementation class is far from obvious, I would
appreciate
> if someone who's familiar with the code can confirm my findings.
>
> The problem occurs in this scenario. First, the generation of the JSP
> implementation
> class for one page fails, due to a missing setter method for a custom
action
> attribute. This results in a CompileException being thrown by the
> TagBeginGenerator
> generateSetters() method. This is the expected behavior.
>
> After this failure, though, processing of *any* page containing a custom
action
> results in invalid code being generated. Example:
>
> com.ora.jsp.tags.counter.IncrementCounterTag
_jspx_th_ora_incrementCounter_0 =
> new com.ora.jsp.tags.counter.IncrementCounterTag();
> _jspx_th_ora_incrementCounter_0.setPageContext(pageContext);
> _jspx_th_ora_incrementCounter_0.setParent(_jspx_th_iob_dbStore_0);
>
> The _jspx_th_iob_dbStore_0 variable refers to the tag handler in the first
page,
> i.e. the one with the missing setter method; it's not defined at all in
this
> page.
>
> The reason is that the state of a static variable in TagGeneratorBase is
busted:
>
> static private Stack tagHandlerStack = new Stack();
>
> This Stack seems to be intended to hold the nesting hierarchy for action
> elements within a page. I have not traced through all code, but it seems
like
> if the code generation goes well, the state of the Stack is maintained by
an
> equal number of push() and pop() calls, but if the code generation fails,
junk
> is left on the Stack.
>
> I can't understand why this is a static variable; making it a regular
instance
> variable should solve the problem since a new instance of TagGeneratorBase
is
> created for each page. Having it as a static probably means that there are
> multi-threading issues as well, even though I haven't run into them yet.
> There's one more static in this class:
>
> static private Hashtable tagVarNumbers = new Hashtable();
>
> I'm not sure how it's used, but it can probably cause similar problems.
>
> My suggestion is to simple make both these variables regular instance
variables.
> If I'm missing something, please let me know.
>
> Hans
> --
> Hans Bergsten hans@gefionsoftware.com
> Gefion Software http://www.gefionsoftware.com
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: tomcat-dev-help@jakarta.apache.org
>
>