You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by "Clinton, Doug" <dc...@tanning.com> on 2000/11/24 15:06:51 UTC
[PATCH] Possible fix for bug #373
Further to my earlier message, I've been investigating the problem with
ajp13 and found the following.
RequestDispatcher.forward() calls realResponse.finish() as its final action.
However, ContextManager.service() also calls finish() on the response which
results in two packets of type JK_AJP13_END_RESPONSE being written back to
the mod_jk stream. This means that the next time mod_jk makes a request the
first thing it will get back from tomcat is the second END_RESPONSE packet.
The real response to the request gets left, buffered in the socket stream.
Hence, every time mod_jk makes a request which must be passed through
forward, the whole reponse stream gets one more step behind.
My first fix to the problem was to remove the call to finish() from
RequestDispatcher.forward(). However, I am unsure as to what possible side
effects this might have, hence I have produced the patch below to
Ajp13ConnectorResponse.java which prevents multiple calls to finish() from
producing more than one END_RESPONSE packet. The flag to check for this is
reset in the recycle() method so that the Ajp13ConnectorResponse can be
reused.
Since this is the first time I've delved into the tomcat code I'm concerned
that I may have missed something critical so if anyone who has more
experience of this area of the code can look over the patch I'd appreciate
it. In particular I'm not sure whether the call to super.finish() should be
made regardless of whether it has been called already.
thanks,
Doug Clinton
dclinton@tannning.com
diff -p Ajp13ConnectorResponse.java Ajp13ConnectorResponse.java.orig
*** Ajp13ConnectorResponse.java Fri Nov 24 13:53:37 2000
--- Ajp13ConnectorResponse.java.orig Tue Nov 21 03:24:06 2000
*************** public class Ajp13ConnectorResponse exte
*** 97,104 ****
MsgConnector con;
- private boolean finished = false;
-
public Ajp13ConnectorResponse()
{
}
--- 97,102 ----
*************** public class Ajp13ConnectorResponse exte
*** 150,165 ****
public void finish() throws IOException
{
! if (!finished) {
! super.finish();
! finished = true;
! MsgBuffer msg = con.getMsgBuffer();
! msg.reset();
! msg.appendByte(JK_AJP13_END_RESPONSE);
! msg.appendByte((byte)1);
! msg.end();
! con.send(msg);
! }
}
protected int headerNameToSc(String name)
--- 148,160 ----
public void finish() throws IOException
{
! super.finish();
! MsgBuffer msg = con.getMsgBuffer();
! msg.reset();
! msg.appendByte(JK_AJP13_END_RESPONSE);
! msg.appendByte((byte)1);
! msg.end();
! con.send(msg);
}
protected int headerNameToSc(String name)
*************** public class Ajp13ConnectorResponse exte
*** 215,221 ****
public void recycle()
{
super.recycle();
- finished = false;
}
public void setConnector(MsgConnector con)
--- 210,215 ----
Re: [PATCH] Possible fix for bug #373
Posted by Gomez Henri <hg...@slib.fr>.
> OK, I have studied Doug's patch and do not see how it can cause any
> potential
> regression problems. Therefore, I've committed the patch to the 3.2
> code base.
> Henri/Doug/others could you please to a fresh checkout of the
> "tomcat_32" branch
> and try this out? There is still time to revert the patch before 3.2
> final if
> it causes any unanticipated problems.
I'll take all the tomcat 3.2 branch tomorrow to build a pre-release RPM.
This patch works well for me but I didn't do exhaustif test. I'll tell you more
tomorrow morning (CET)
Regards
Re: [PATCH] Possible fix for bug #373
Posted by "Craig R. McClanahan" <Cr...@eng.sun.com>.
OK, I have studied Doug's patch and do not see how it can cause any potential
regression problems. Therefore, I've committed the patch to the 3.2 code base.
Henri/Doug/others could you please to a fresh checkout of the "tomcat_32" branch
and try this out? There is still time to revert the patch before 3.2 final if
it causes any unanticipated problems.
Craig McClanahan
"Clinton, Doug" wrote:
> Further to my earlier message, I've been investigating the problem with
> ajp13 and found the following.
>
> RequestDispatcher.forward() calls realResponse.finish() as its final action.
> However, ContextManager.service() also calls finish() on the response which
> results in two packets of type JK_AJP13_END_RESPONSE being written back to
> the mod_jk stream. This means that the next time mod_jk makes a request the
> first thing it will get back from tomcat is the second END_RESPONSE packet.
> The real response to the request gets left, buffered in the socket stream.
>
> Hence, every time mod_jk makes a request which must be passed through
> forward, the whole reponse stream gets one more step behind.
>
> My first fix to the problem was to remove the call to finish() from
> RequestDispatcher.forward(). However, I am unsure as to what possible side
> effects this might have, hence I have produced the patch below to
> Ajp13ConnectorResponse.java which prevents multiple calls to finish() from
> producing more than one END_RESPONSE packet. The flag to check for this is
> reset in the recycle() method so that the Ajp13ConnectorResponse can be
> reused.
>
> Since this is the first time I've delved into the tomcat code I'm concerned
> that I may have missed something critical so if anyone who has more
> experience of this area of the code can look over the patch I'd appreciate
> it. In particular I'm not sure whether the call to super.finish() should be
> made regardless of whether it has been called already.
>
> thanks,
> Doug Clinton
> dclinton@tannning.com
>
> diff -p Ajp13ConnectorResponse.java Ajp13ConnectorResponse.java.orig
> *** Ajp13ConnectorResponse.java Fri Nov 24 13:53:37 2000
> --- Ajp13ConnectorResponse.java.orig Tue Nov 21 03:24:06 2000
> *************** public class Ajp13ConnectorResponse exte
> *** 97,104 ****
>
> MsgConnector con;
>
> - private boolean finished = false;
> -
> public Ajp13ConnectorResponse()
> {
> }
> --- 97,102 ----
> *************** public class Ajp13ConnectorResponse exte
> *** 150,165 ****
>
> public void finish() throws IOException
> {
> ! if (!finished) {
> ! super.finish();
> ! finished = true;
> ! MsgBuffer msg = con.getMsgBuffer();
> ! msg.reset();
> ! msg.appendByte(JK_AJP13_END_RESPONSE);
> ! msg.appendByte((byte)1);
> ! msg.end();
> ! con.send(msg);
> ! }
> }
>
> protected int headerNameToSc(String name)
> --- 148,160 ----
>
> public void finish() throws IOException
> {
> ! super.finish();
> ! MsgBuffer msg = con.getMsgBuffer();
> ! msg.reset();
> ! msg.appendByte(JK_AJP13_END_RESPONSE);
> ! msg.appendByte((byte)1);
> ! msg.end();
> ! con.send(msg);
> }
>
> protected int headerNameToSc(String name)
> *************** public class Ajp13ConnectorResponse exte
> *** 215,221 ****
> public void recycle()
> {
> super.recycle();
> - finished = false;
> }
>
> public void setConnector(MsgConnector con)
> --- 210,215 ----