Rev 4514: lifeless said: try harder :) in file:///home/vila/src/bzr/bugs/383920-pycurl-hangs/

[Vincent Ladeuil]

At file:///home/vila/src/bzr/bugs/383920-pycurl-hangs/

------------------------------------------------------------
revno: 4514
revision-id: v.ladeuil+lp at free.fr-20090708085119-rmfk350eoiwxtpqh
parent: v.ladeuil+lp at free.fr-20090707082457-otq7zht7lgkpnpbr
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: 383920-pycurl-hangs
timestamp: Wed 2009-07-08 10:51:19 +0200
message:
  lifeless said: try harder :)
  
  * bzrlib/tests/http_server.py:
  (TestingHTTPRequestHandler.send_error): Deleted, not needed
  anymre.
  (TestingThreadingHTTPServer.process_request_thread): shutdown()
  the socket explicitely.
  
  * NEWS: 
  Describe the new fix.
-------------- next part --------------
=== modified file 'NEWS'
--- a/NEWS	2009-07-07 08:24:57 +0000
+++ b/NEWS	2009-07-08 08:51:19 +0000
@@ -75,14 +75,13 @@
   of a remote repository.
   (Jelmer Vernooij, #332194)
 
-* Fix bogus ``send_error`` in python's BaseHTTPServer by inserting the missing
-  'Content-Length header. https pycurl clients can hang otherwise.
-  (Vincent Ladeuil, #383920).
-
 * Force deletion of readonly files during merge, update and other tree
   transforms.
   (Craig Hewetson, Martin Pool, #218206)
 
+* Force socket shutdown in threaded http test servers to avoid client hangs
+  (pycurl).  (Vincent Ladeuil, #383920).
+
 * The ``log+`` decorator, useful in debugging or profiling, could cause
   "AttributeError: 'list' object has no attribute 'next'".  This is now
   fixed.  The log decorator no longer shows the elapsed time or transfer

=== modified file 'bzrlib/tests/http_server.py'
--- a/bzrlib/tests/http_server.py	2009-07-07 08:24:57 +0000
+++ b/bzrlib/tests/http_server.py	2009-07-08 08:51:19 +0000
@@ -145,35 +145,6 @@
 
         return SimpleHTTPServer.SimpleHTTPRequestHandler.send_head(self)
 
-    def send_error(self, code, message=None):
-        """Send and log an error reply.
-
-        Replace the bogus BaseHTTPServer.py with one that add the
-        Content-Length header to address strict http clients.
-        """
-
-        try:
-            short, long = self.responses[code]
-        except KeyError:
-            short, long = '???', '???'
-        if message is None:
-            message = short
-        explain = long
-        self.log_error("code %d, message %s", code, message)
-        # using _quote_html to prevent Cross Site Scripting attacks (see bug
-        # #1100201)
-        content = (self.error_message_format %
-                   {'code': code,
-                    'message': BaseHTTPServer._quote_html(message),
-                    'explain': explain})
-        self.send_response(code, message)
-        self.send_header("Content-Length", "%d" % len(content))
-        self.send_header("Content-Type", self.error_content_type)
-        self.send_header('Connection', 'close')
-        self.end_headers()
-        if self.command != 'HEAD' and code >= 200 and code not in (204, 304):
-            self.wfile.write(content)
-
     def send_range_content(self, file, start, length):
         file.seek(start)
         self.wfile.write(file.read(length))
@@ -412,6 +383,23 @@
         # lying around.
         self.daemon_threads = True
 
+    def process_request_thread(self, request, client_address):
+        SocketServer.ThreadingTCPServer.process_request_thread(
+            self, request, client_address)
+        # Under some circumstances (as in bug #383920), we need to force the
+        # shutdown as python delays it until gc occur otherwise and the client
+        # may hang.
+        try:
+            # The request process has been completed, the thread is about to
+            # die, let's shutdown the socket if we can.
+            request.shutdown(socket.SHUT_RDWR)
+        except (socket.error, select.error), e:
+            if e[0] in (errno.EBADF, errno.ENOTCONN):
+                # Right, the socket is already down
+                pass
+            else:
+                raise
+
 
 class HttpServer(transport.Server):
     """A test server for http transports.

=== modified file 'bzrlib/tests/test_read_bundle.py'
--- a/bzrlib/tests/test_read_bundle.py	2009-07-07 08:24:57 +0000
+++ b/bzrlib/tests/test_read_bundle.py	2009-07-08 08:51:19 +0000
@@ -90,7 +90,7 @@
         # read_mergeable_from_url will invoke get_transport which may *not*
         # respect self._transport (i.e. returns a transport that is different
         # from the one we want to test, so we must inject a correct transport
-        # into possible_transports first.
+        # into possible_transports first).
         self.possible_transports = [self.get_transport(self.bundle_name)]
         self._captureVar('BZR_NO_SMART_VFS', None)
         wt = self.create_test_bundle()