Mailing-List: contact tomcat-dev-help@jakarta.apache.org; run by ezmlm
Message-ID: <3A242E92.C5B6C2C@eng.sun.com>
Date: Tue, 28 Nov 2000 14:15:46 -0800
From: "Craig R. McClanahan" <Craig.McClanahan@eng.sun.com>
MIME-Version: 1.0
To: tomcat-dev@jakarta.apache.org
Subject: Re: [PATCH] Possible fix for bug #373
References: <4115CB6C1B39D21198470008C71EC6CD010669C0@tte-nt2>
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit

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 ----