You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2011/10/19 22:55:39 UTC

svn commit: r1186485 - in /tomcat/tc7.0.x/trunk: ./ java/org/apache/coyote/http11/AbstractHttp11Processor.java webapps/docs/changelog.xml

Author: markt
Date: Wed Oct 19 20:55:39 2011
New Revision: 1186485

URL: http://svn.apache.org/viewvc?rev=1186485&view=rev
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=52055
Buffers not correctly reset between keep-alive requests when using async

Modified:
    tomcat/tc7.0.x/trunk/   (props changed)
    tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java
    tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml

Propchange: tomcat/tc7.0.x/trunk/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Wed Oct 19 20:55:39 2011
@@ -1 +1 @@
-/tomcat/trunk:1156115,1156171,1156276,1156304,1156519,1156530,1156602,1157015,1157018,1157151,1157198,1157204,1157810,1157832,1157834,1157847,1157908,1157939,1158155,1158160,1158176,1158195,1158198-1158199,1158227,1158331,1158334-1158335,1158426,1160347,1160592,1160611,1160619,1160626,1160639,1160652,1160720-1160721,1160772,1160774,1160776,1161303,1161310,1161322,1161339,1161486,1161540,1161549,1161584,1162082,1162149,1162169,1162721,1162769,1162836,1162932,1163630,1164419,1164438,1164469,1164480,1164567,1165234,1165247-1165248,1165253,1165273,1165282,1165309,1165331,1165338,1165347,1165360-1165361,1165367-1165368,1165602,1165608,1165677,1165693,1165721,1165723,1165728,1165730,1165738,1165746,1165765,1165777,1165918,1165921,1166077,1166150-1166151,1166290,1166366,1166620,1166686,1166693,1166752,1166757,1167368,1167394,1169447,1170647,1171692,1172233-1172234,1172236,1172269,1172278,1172282,1172556,1172610,1172664,1172689,1172711,1173020-1173021,1173082,1173088,1173090,1173096
 ,1173241,1173256,1173288,1173333,1173342,1173461,1173614,1173630,1173659,1173722,1174061,1174239,1174322,1174325,1174329-1174330,1174337-1174339,1174343,1174353,1174799,1174882,1174884,1174983,1175155,1175158,1175167,1175182,1175190,1175201,1175272,1175275,1175283,1175582,1175589-1175590,1175594,1175602,1175613,1175633,1175690,1175713,1175889,1175896,1175907,1176584,1176590,1176799,1177050,1177060,1177125,1177152,1177160,1177245,1177850,1177862,1177978,1178209,1178228,1178233,1178449,1178542,1178681,1178684,1178721,1179268,1179274,1180261,1180865,1180891,1180894,1180907,1181028,1181123,1181125,1181136,1181291,1181743,1182796,1183078,1183105,1183142,1183328,1183339-1183340,1183492-1183494,1183605,1184917,1184919,1185018,1185020,1185200,1185588,1185626,1185756,1185758,1186011,1186042-1186045,1186104,1186123,1186137,1186153,1186254,1186257,1186377-1186379,1186479
+/tomcat/trunk:1156115,1156171,1156276,1156304,1156519,1156530,1156602,1157015,1157018,1157151,1157198,1157204,1157810,1157832,1157834,1157847,1157908,1157939,1158155,1158160,1158176,1158195,1158198-1158199,1158227,1158331,1158334-1158335,1158426,1160347,1160592,1160611,1160619,1160626,1160639,1160652,1160720-1160721,1160772,1160774,1160776,1161303,1161310,1161322,1161339,1161486,1161540,1161549,1161584,1162082,1162149,1162169,1162721,1162769,1162836,1162932,1163630,1164419,1164438,1164469,1164480,1164567,1165234,1165247-1165248,1165253,1165273,1165282,1165309,1165331,1165338,1165347,1165360-1165361,1165367-1165368,1165602,1165608,1165677,1165693,1165721,1165723,1165728,1165730,1165738,1165746,1165765,1165777,1165918,1165921,1166077,1166150-1166151,1166290,1166366,1166620,1166686,1166693,1166752,1166757,1167368,1167394,1169447,1170647,1171692,1172233-1172234,1172236,1172269,1172278,1172282,1172556,1172610,1172664,1172689,1172711,1173020-1173021,1173082,1173088,1173090,1173096
 ,1173241,1173256,1173288,1173333,1173342,1173461,1173614,1173630,1173659,1173722,1174061,1174239,1174322,1174325,1174329-1174330,1174337-1174339,1174343,1174353,1174799,1174882,1174884,1174983,1175155,1175158,1175167,1175182,1175190,1175201,1175272,1175275,1175283,1175582,1175589-1175590,1175594,1175602,1175613,1175633,1175690,1175713,1175889,1175896,1175907,1176584,1176590,1176799,1177050,1177060,1177125,1177152,1177160,1177245,1177850,1177862,1177978,1178209,1178228,1178233,1178449,1178542,1178681,1178684,1178721,1179268,1179274,1180261,1180865,1180891,1180894,1180907,1181028,1181123,1181125,1181136,1181291,1181743,1182796,1183078,1183105,1183142,1183328,1183339-1183340,1183492-1183494,1183605,1184917,1184919,1185018,1185020,1185200,1185588,1185626,1185756,1185758,1186011,1186042-1186045,1186104,1186123,1186137,1186153,1186254,1186257,1186377-1186379,1186479-1186480

Modified: tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java?rev=1186485&r1=1186484&r2=1186485&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java (original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java Wed Oct 19 20:55:39 2011
@@ -1538,6 +1538,8 @@ public abstract class AbstractHttp11Proc
             if (!keepAlive) {
                 return SocketState.CLOSED;
             } else {
+                getInputBuffer().nextRequest();
+                getOutputBuffer().nextRequest();
                 return SocketState.OPEN;
             }
         }

Modified: tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml?rev=1186485&r1=1186484&r2=1186485&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml Wed Oct 19 20:55:39 2011
@@ -110,6 +110,11 @@
         authentication from being lost during the authentication process.
         (markt)
       </fix>
+      <fix>
+        <bug>52055</bug>: Ensure that the input and output buffers are correctly
+        reset between keep-alive requests when using Servlet 3.0 asynchronous
+        request processing. (markt)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Coyote">



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


Re: svn commit: r1186485 - in /tomcat/tc7.0.x/trunk: ./ java/org/apache/coyote/http11/AbstractHttp11Processor.java webapps/docs/changelog.xml

Posted by Mark Thomas <ma...@apache.org>.
On 20/10/2011 10:07, Konstantin Kolinko wrote:
> 2011/10/20  <ma...@apache.org>:
>> Author: markt
>> Date: Wed Oct 19 20:55:39 2011
>> New Revision: 1186485
>>
>> URL: http://svn.apache.org/viewvc?rev=1186485&view=rev
>> Log:
>> Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=52055
>> Buffers not correctly reset between keep-alive requests when using async

<snip/>

> Searching for places that use org.apache.coyote.Constants.STAGE_ENDED ...

SocketState.OPEN is probably a clearer indication of this particular
circumstance.

> 1) it looks like AbstractAjpProcessor#asyncDispatch(SocketStatus)
> needs similar patch:
> it has "return SocketState.OPEN;"
> 
> If I understand correctly in Ajp that will be recycle(false)

This already happens. asyncDispatch() is called from
AbstractProtocol$AbstractConnectionHandler which will call release()
when SocketState.OPEN is returned which in turn calls
processor.recycle(false)

> 2) In Http11NioProcessor#event() I do not see nextRequest() calls, but
> similar implementation in Http11AprProcessor#event() has nextRequest()
> calls.

For APR on CLOSED, I don't see that the nextRequest() calls are doing
anything useful. The processor is going to get recycled which will
recycle the buffers. There is no need for nextRequest() in this case.

For NIO on OPEN, I think there is a case for nextRequest(). I'll add the
necessary calls.

<aside>
The more similar the connectors, the easier this type of code review
becomes. I would suggest that this supports the view that refactoring
the connections to align the code is a good thing.

I think there is still scope to further align code both across the
BIO/NIO/APR boundary but also across the HTTP/AJP boundary.
</aside>

Mark

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


Re: svn commit: r1186485 - in /tomcat/tc7.0.x/trunk: ./ java/org/apache/coyote/http11/AbstractHttp11Processor.java webapps/docs/changelog.xml

Posted by Konstantin Kolinko <kn...@gmail.com>.
2011/10/20  <ma...@apache.org>:
> Author: markt
> Date: Wed Oct 19 20:55:39 2011
> New Revision: 1186485
>
> URL: http://svn.apache.org/viewvc?rev=1186485&view=rev
> Log:
> Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=52055
> Buffers not correctly reset between keep-alive requests when using async
>
> Modified:
>    tomcat/tc7.0.x/trunk/   (props changed)
>    tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java
>    tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml
>

> --- tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java (original)
> +++ tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java Wed Oct 19 20:55:39 2011
> @@ -1538,6 +1538,8 @@ public abstract class AbstractHttp11Proc
>             if (!keepAlive) {
>                 return SocketState.CLOSED;
>             } else {
> +                getInputBuffer().nextRequest();
> +                getOutputBuffer().nextRequest();
>                 return SocketState.OPEN;
>             }
>         }
>


Searching for places that use org.apache.coyote.Constants.STAGE_ENDED ...

1) it looks like AbstractAjpProcessor#asyncDispatch(SocketStatus)
needs similar patch:
it has "return SocketState.OPEN;"

If I understand correctly in Ajp that will be recycle(false)

2) In Http11NioProcessor#event() I do not see nextRequest() calls, but
similar implementation in Http11AprProcessor#event() has nextRequest()
calls.


Best regards,
Konstantin Kolinko

> --- tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml (original)
> +++ tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml Wed Oct 19 20:55:39 2011
> @@ -110,6 +110,11 @@
>         authentication from being lost during the authentication process.
>         (markt)
>       </fix>
> +      <fix>
> +        <bug>52055</bug>: Ensure that the input and output buffers are correctly
> +        reset between keep-alive requests when using Servlet 3.0 asynchronous
> +        request processing. (markt)
> +      </fix>
>     </changelog>
>   </subsection>
>   <subsection name="Coyote">
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>

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