[PATCH 1 of 6] Fix test-push-http: Workaround for missing shutdown in SocketServer.TCPServer.close_request

[Mads Kiilerich]

Dirkjan Ochtman wrote:
> Mads Kiilerich <mads <at> kiilerich.com> writes:
>   
>> +        def close_request(self, request):
>> +            """Fix missing shutdown in SocketServer.TCPServer.close_request
>> +            which causes socket.close() to send RST (on some OSs), causing 
>> +            recv() in hg push to (sometimes) fail even with data waiting."""
>> +            request.shutdown(socket.SHUT_WR)
>> +            BaseHTTPServer.HTTPServer.close_request(self, request)
>>     
>
> Hmm, not sure about this one. How did you come up with this? Of what type is
> the request parameter for close_request()? Also HTTPServer.close_request() seems
> to actually be TCPServer.close_request(), which calls request.close(). Would that
> not be enough, or do we actually need to shut the socket down?
>
> It seems rather obscure to me.
>   

Indeed it is obscure. Black magic.

The patch could as well have looked like:
--- /usr/lib/python2.5/SocketServer.py    2008-09-18 15:16:45.000000000
+0200
+++ ./SocketServer.py    2008-09-18 15:16:36.000000000 +0200
@@ -375,6 +375,7 @@
 
     def close_request(self, request):
         """Called to clean up an individual request."""
+        request.shutdown(socket.SHUT_WR)
         request.close()
 
- but that is out of our scope ... I don't know if it is a bug in
SocketServer or we just used it in a wrong way somehow.

But I'm not sure it is the right fix. It might have undesired side
effects. But it works for me. An expert should give a second opinion.
Any experts around?

Some entertainment on the topic can be found with
http://www.google.com/search?q=tcp+rst and
http://www.google.com/search?q=socket+shutdown+before+close

/Mads
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://selenic.com/pipermail/mercurial-devel/attachments/20080918/1b24a6f5/attachment.htm