tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ma...@apache.org
Subject Re: Review Request: Proposed fix for BZ 50903
Date Tue, 15 Mar 2011 12:54:08 GMT


> On 2011-03-14 10:07:37, Filip Hanik wrote:
> > I think the entire solution is over complicated. Not a fan of introducing the processor
into the input buffer, for an edge case. 
> > If you are stopping the connector, I would let the current request finish up.
> > Since Tomcat 7 should bring the connection back to the endpoint in between keep
alive requests, let the end point decide not to continue with the next request.
> > I don't think modifying each input buffer to check if the end point is paused. Instead,
I would delegate this responsibility to the endpoint to make the decision in between requests.

With this patch, current requests are allowed to finish. What the patch handles is threads
that are in keep-alive blocking waiting for the next request from the client. Prior to the
patch the sequence is:
thread enters keep-alive (blocks waiting for data), connector stopped, client sends data,
request is processed
After the patch the sequence is:
thread enters keep-alive (blocks waiting for data), connector stopped, client sends data,
request is rejected
Currently processing requests then the connector is stopped are unaffected.

All that said, I take your point. I'll see if I can come up with something better. On reflection
I would also like to differentiate between Connector.pause() (let current requests complete)
and connector stop() (forcibly end current requests). There are definite use cases where this
would help our users so I would like to get this into Tomcat 7 in some form.


- markt


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/500/#review328
-----------------------------------------------------------


On 2011-03-13 16:05:12, markt wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/500/
> -----------------------------------------------------------
> 
> (Updated 2011-03-13 16:05:12)
> 
> 
> Review request for tomcat.
> 
> 
> Summary
> -------
> 
> Fix for https://issues.apache.org/bugzilla/show_bug.cgi?id=50903
> Currently, if a connector is stopped with an open keep-alive connection, the next request
received on that connection will be processed. This patch changes that so the request is not
processed and the socket closed.
> 
> 
> This addresses bug https://issues.apache.org/bugzilla/show_bug.cgi?id=50903.
>     https://issues.apache.org/jira/browse/https://issues.apache.org/bugzilla/show_bug.cgi?id=50903
> 
> 
> Diffs
> -----
> 
>   /java/org/apache/coyote/ajp/AbstractAjpProcessor.java 1081214 
>   /java/org/apache/coyote/ajp/AjpAprProcessor.java 1081214 
>   /java/org/apache/coyote/ajp/AjpProcessor.java 1081214 
>   /java/org/apache/coyote/ajp/LocalStrings.properties 1081214 
>   /java/org/apache/coyote/http11/AbstractInputBuffer.java 1081214 
>   /java/org/apache/coyote/http11/Http11AprProcessor.java 1081214 
>   /java/org/apache/coyote/http11/Http11NioProcessor.java 1081214 
>   /java/org/apache/coyote/http11/Http11Processor.java 1081214 
>   /java/org/apache/coyote/http11/InternalAprInputBuffer.java 1081214 
>   /java/org/apache/coyote/http11/InternalInputBuffer.java 1081214 
>   /java/org/apache/coyote/http11/InternalNioInputBuffer.java 1081214 
>   /java/org/apache/coyote/http11/LocalStrings.properties 1081214 
>   /test/org/apache/catalina/connector/TestConnector.java PRE-CREATION 
>   /test/org/apache/catalina/startup/TesterServlet.java PRE-CREATION 
>   /test/org/apache/catalina/startup/TomcatBaseTest.java 1081214 
>   /test/org/apache/coyote/http11/TestTbd.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/500/diff
> 
> 
> Testing
> -------
> 
> Includes unit test (executed with BIO)
> Manually tested with all 5 connectors.
> 
> 
> Thanks,
> 
> markt
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message