Rev 4519: (vila) Force socket shutdown in threaded http test servers in file:///home/pqm/archives/thelove/bzr/%2Btrunk/

Canonical.com Patch Queue Manager pqm at pqm.ubuntu.com
Wed Jul 8 19:05:47 BST 2009


At file:///home/pqm/archives/thelove/bzr/%2Btrunk/

------------------------------------------------------------
revno: 4519 [merge]
revision-id: pqm at pqm.ubuntu.com-20090708180538-ars9cga8c3p5tcgz
parent: pqm at pqm.ubuntu.com-20090708164255-irg4jbg3bsf69vvg
parent: v.ladeuil+lp at free.fr-20090708152458-l2pej4mhob05n08f
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Wed 2009-07-08 19:05:38 +0100
message:
  (vila) Force socket shutdown in threaded http test servers
modified:
  NEWS                           NEWS-20050323055033-4e00b5db738777ff
  bzrlib/tests/http_server.py    httpserver.py-20061012142527-m1yxdj1xazsf8d7s-1
  bzrlib/tests/test_read_bundle.py test_read_bundle.py-20060615211421-ud8cwr1ulgd914zf-1
  bzrlib/tests/test_transport_implementations.py test_transport_implementations.py-20051227111451-f97c5c7d5c49fce7
=== modified file 'NEWS'
--- a/NEWS	2009-07-08 14:40:29 +0000
+++ b/NEWS	2009-07-08 18:05:38 +0000
@@ -79,6 +79,9 @@
   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-03-23 14:59:43 +0000
+++ b/bzrlib/tests/http_server.py	2009-07-08 15:24:31 +0000
@@ -382,6 +382,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-03-23 14:59:43 +0000
+++ b/bzrlib/tests/test_read_bundle.py	2009-07-08 08:51:19 +0000
@@ -84,70 +84,54 @@
 class TestReadBundleFromURL(TestTransportImplementation):
     """Test that read_bundle works properly across multiple transports"""
 
+    def setUp(self):
+        super(TestReadBundleFromURL, self).setUp()
+        self.bundle_name = 'test_bundle'
+        # 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).
+        self.possible_transports = [self.get_transport(self.bundle_name)]
+        self._captureVar('BZR_NO_SMART_VFS', None)
+        wt = self.create_test_bundle()
+
+    def read_mergeable_from_url(self, url):
+        return bzrlib.bundle.read_mergeable_from_url(
+            url, possible_transports=self.possible_transports)
+
     def get_url(self, relpath=''):
         return bzrlib.urlutils.join(self._server.get_url(), relpath)
 
     def create_test_bundle(self):
         out, wt = create_bundle_file(self)
         if self.get_transport().is_readonly():
-            f = open('test_bundle', 'wb')
-            try:
-                f.write(out.getvalue())
-            finally:
-                f.close()
+            self.build_tree_contents([(self.bundle_name, out.getvalue())])
         else:
-            self.get_transport().put_file('test_bundle', out)
-            self.log('Put to: %s', self.get_url('test_bundle'))
+            self.get_transport().put_file(self.bundle_name, out)
+            self.log('Put to: %s', self.get_url(self.bundle_name))
         return wt
 
     def test_read_mergeable_from_url(self):
-        self._captureVar('BZR_NO_SMART_VFS', None)
-        wt = self.create_test_bundle()
-        if wt is None:
-            return
-        # 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.
-        t = self.get_transport('test_bundle')
-        possible_transports = [t]
-        info = bzrlib.bundle.read_mergeable_from_url(
-                    unicode(self.get_url('test_bundle')),
-                    possible_transports=possible_transports)
+        info = self.read_mergeable_from_url(
+            unicode(self.get_url(self.bundle_name)))
         revision = info.real_revisions[-1]
         self.assertEqual('commit-1', revision.revision_id)
 
     def test_read_fail(self):
         # Trying to read from a directory, or non-bundle file
         # should fail with NotABundle
-        self._captureVar('BZR_NO_SMART_VFS', None)
-        wt = self.create_test_bundle()
-        if wt is None:
-            return
-
-        self.assertRaises(errors.NotABundle,
-            bzrlib.bundle.read_mergeable_from_url,
-            self.get_url('tree'))
-        self.assertRaises(errors.NotABundle,
-            bzrlib.bundle.read_mergeable_from_url,
-            self.get_url('tree/a'))
+        self.assertRaises(errors.NotABundle,
+                          self.read_mergeable_from_url, self.get_url('tree'))
+        self.assertRaises(errors.NotABundle,
+                          self.read_mergeable_from_url, self.get_url('tree/a'))
 
     def test_read_mergeable_respects_possible_transports(self):
-        t = self.get_transport('test_bundle')
-        if not isinstance(t, bzrlib.transport.ConnectedTransport):
+        if not isinstance(self.get_transport(self.bundle_name),
+                          bzrlib.transport.ConnectedTransport):
             # There is no point testing transport reuse for not connected
             # transports (the test will fail even).
-            return
-        self._captureVar('BZR_NO_SMART_VFS', None)
-        wt = self.create_test_bundle()
-        if wt is None:
-            return
-        # 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.
-        possible_transports = [t]
-        url = unicode(self.get_url('test_bundle'))
-        info = bzrlib.bundle.read_mergeable_from_url(url,
-            possible_transports=possible_transports)
-        self.assertEqual(1, len(possible_transports))
+            raise tests.TestSkipped(
+                'Need a ConnectedTransport to test transport reuse')
+        url = unicode(self.get_url(self.bundle_name))
+        info = self.read_mergeable_from_url(url)
+        self.assertEqual(1, len(self.possible_transports))

=== modified file 'bzrlib/tests/test_transport_implementations.py'
--- a/bzrlib/tests/test_transport_implementations.py	2009-04-20 04:19:45 +0000
+++ b/bzrlib/tests/test_transport_implementations.py	2009-07-07 08:24:57 +0000
@@ -203,6 +203,13 @@
         for content, f in itertools.izip(contents, content_f):
             self.assertEqual(content, f.read())
 
+    def test_get_unknown_file(self):
+        t = self.get_transport()
+        files = ['a', 'b']
+        contents = ['contents of a\n',
+                    'contents of b\n',
+                    ]
+        self.build_tree(files, transport=t, line_endings='binary')
         self.assertRaises(NoSuchFile, t.get, 'c')
         self.assertListRaises(NoSuchFile, t.get_multi, ['a', 'b', 'c'])
         self.assertListRaises(NoSuchFile, t.get_multi, iter(['a', 'b', 'c']))
@@ -242,6 +249,9 @@
         for content, fname in zip(contents, files):
             self.assertEqual(content, t.get_bytes(fname))
 
+    def test_get_bytes_unknown_file(self):
+        t = self.get_transport()
+
         self.assertRaises(NoSuchFile, t.get_bytes, 'c')
 
     def test_get_with_open_write_stream_sees_all_content(self):




More information about the bazaar-commits mailing list