Review Board 1.7.22


Proposed fix for BZ 50903

Review Request #500 - Created March 13, 2011 and submitted

markt
trunk
https://issues.apache.org/bugzilla/show_bug.cgi?id=50903
Reviewers
tomcat
tomcat-7.0.x
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.
Includes unit test (executed with BIO)
Manually tested with all 5 connectors.
Posted (March 13, 2011, 4:08 p.m.)

   

  
Ignore this file. It is a test case for a separate bug
Posted (March 14, 2011, 10:07 a.m.)
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.
  1. 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.