Rev 5259: Ensure that all transports close their underlying connection. in file:///home/vila/src/bzr/experimental/leaking-tests/

Vincent Ladeuil v.ladeuil+lp at free.fr
Tue Jun 1 14:01:20 BST 2010


At file:///home/vila/src/bzr/experimental/leaking-tests/

------------------------------------------------------------
revno: 5259
revision-id: v.ladeuil+lp at free.fr-20100601130120-yiv1mdl2wtzsn8mm
parent: v.ladeuil+lp at free.fr-20100531213128-42yhxx5q4motgh3h
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: catch-them-all
timestamp: Tue 2010-06-01 15:01:20 +0200
message:
  Ensure that all transports close their underlying connection.
  
  * bzrlib/transport/sftp.py:
  (SFTPTransport.disconnect): Close the sftp connection.
  
  * bzrlib/transport/remote.py:
  (RemoteTransport.disconnect): The medium can be None.
  
  * bzrlib/transport/http/_urllib.py:
  (HttpTransport_urllib.disconnect): Close the http connection.
  
  * bzrlib/transport/http/_pycurl.py:
  (PyCurlTransport.disconnect): Close the Curl object.
  
  * bzrlib/transport/http/__init__.py:
  (SmartClientHTTPMedium.disconnect): Disconnect the backing http
  transport.
  
  * bzrlib/transport/gio_transport.py:
  (GioTransport.disconnect): Hmm, nothing obvious here.
  
  * bzrlib/transport/ftp/__init__.py:
  (FtpTransport.disconnect): Close the ftplib connection.
  
  * bzrlib/transport/__init__.py:
  (Transport.disconnect): A do-nothing implementation by default.
  (ConnectedTransport.disconnect): A new required method.
  
  * bzrlib/tests/test_smart_transport.py:
  (TestSmartTCPServer.test_get_error_unexpected)
  (SmartTCPTests.start_server): Revert to SmartServer waiting for a
  better way to capture the connections.
  
  * bzrlib/tests/per_transport.py:
  (TransportTests.test_connection_sharing): 'None' is better than
  'object()' for a dummy connection.
  
  * bzrlib/tests/__init__.py:
  (TestCase._check_leaked_threads): Don't fail at first leak or all
  subsequent tests will fail too. A more elaborate scheme would be
  to use threading.enumerate() but that's a big hammer with little
  benefits as long as there exist "valid" leaks (like the smart
  server).
  (TestCaseWithMemoryTransport.setUp): Make sure we call
  transport.disconnect() for all created transports.
-------------- next part --------------
=== modified file 'bzrlib/tests/__init__.py'
--- a/bzrlib/tests/__init__.py	2010-05-31 21:31:28 +0000
+++ b/bzrlib/tests/__init__.py	2010-06-01 13:01:20 +0000
@@ -831,7 +831,6 @@
 
     def _check_leaked_threads(self):
         active = threading.activeCount()
-        self.assertEqual(1, active, '%r is leaking thread(s)' % self.id())
         leaked_threads = active - TestCase._active_threads
         TestCase._active_threads = active
         # If some tests make the number of threads *decrease*, we'll consider
@@ -2432,6 +2431,15 @@
 
     def setUp(self):
         super(TestCaseWithMemoryTransport, self).setUp()
+        # Ensure that ConnectedTransport doesn't leak sockets
+        def get_transport_with_cleanup(*args, **kwargs):
+            t = self._orig_get_transport(*args, **kwargs)
+            if isinstance(t, _mod_transport.ConnectedTransport):
+                self.addCleanup(t.disconnect)
+            return t
+
+        self._orig_get_transport = self.overrideAttr(
+            _mod_transport, 'get_transport', get_transport_with_cleanup)
         self._make_test_root()
         self.addCleanup(os.chdir, os.getcwdu())
         self.makeAndChdirToTestDir()

=== modified file 'bzrlib/tests/per_transport.py'
--- a/bzrlib/tests/per_transport.py	2010-03-05 05:30:19 +0000
+++ b/bzrlib/tests/per_transport.py	2010-06-01 13:01:20 +0000
@@ -1265,7 +1265,7 @@
         self.assertIs(t._get_connection(), c._get_connection())
 
         # Temporary failure, we need to create a new dummy connection
-        new_connection = object()
+        new_connection = None
         t._set_connection(new_connection)
         # Check that both transports use the same connection
         self.assertIs(new_connection, t._get_connection())

=== modified file 'bzrlib/tests/test_smart_transport.py'
--- a/bzrlib/tests/test_smart_transport.py	2010-05-31 15:29:23 +0000
+++ b/bzrlib/tests/test_smart_transport.py	2010-06-01 13:01:20 +0000
@@ -41,7 +41,6 @@
         vfs,
 )
 from bzrlib.tests import (
-    test_server,
     test_smart,
     )
 from bzrlib.transport import (
@@ -972,8 +971,7 @@
                 return self.base
             def get_bytes(self, path):
                 raise Exception("some random exception from inside server")
-        smart_server = test_server.SmartTCPServer_for_testing(
-            backing_transport=FlakyTransport())
+        smart_server = server.SmartTCPServer(backing_transport=FlakyTransport())
         smart_server.start_background_thread('-' + self.id())
         try:
             transport = remote.RemoteTCPTransport(smart_server.get_url())
@@ -1015,8 +1013,7 @@
             self.real_backing_transport = self.backing_transport
             self.backing_transport = transport.get_transport(
                 "readonly+" + self.backing_transport.abspath('.'))
-        self.server = test_server.SmartTCPServer_for_testing(
-            self.backing_transport)
+        self.server = server.SmartTCPServer(self.backing_transport)
         # XXX: Shouldn't we calling self.server.start_server below instead of
         # start_background_thread ? -- vila 20100531
         self.server.start_background_thread('-' + self.id())

=== modified file 'bzrlib/transport/__init__.py'
--- a/bzrlib/transport/__init__.py	2010-05-30 16:26:11 +0000
+++ b/bzrlib/transport/__init__.py	2010-06-01 13:01:20 +0000
@@ -1286,6 +1286,12 @@
         # should be asked to ConnectedTransport only.
         return None
 
+    def disconnect(self):
+        # This is really needed for ConnectedTransport only, but it's easier to
+        # have Transport do nothing than testing that the disconnect should be
+        # asked to ConnectedTransport only.
+        pass
+
     def _redirected_to(self, source, target):
         """Returns a transport suitable to re-issue a redirected request.
 
@@ -1550,6 +1556,13 @@
             transport = self.__class__(other_base, _from_transport=self)
         return transport
 
+    def disconnect(self):
+        """Disconnect the transport.
+
+        If and when required the transport willl reconnect automatically.
+        """
+        raise NotImplementedError(self.disconnect)
+
 
 # We try to recognize an url lazily (ignoring user, password, etc)
 _urlRE = re.compile(r'^(?P<proto>[^:/\\]+)://(?P<rest>.*)$')

=== modified file 'bzrlib/transport/ftp/__init__.py'
--- a/bzrlib/transport/ftp/__init__.py	2010-02-27 01:37:54 +0000
+++ b/bzrlib/transport/ftp/__init__.py	2010-06-01 13:01:20 +0000
@@ -164,6 +164,11 @@
         connection, credentials = self._create_connection(credentials)
         self._set_connection(connection, credentials)
 
+    def disconnect(self):
+        connection = self._get_connection()
+        if connection is not None:
+            connection.close()
+
     def _translate_ftp_error(self, err, path, extra=None,
                               unknown_exc=FtpPathError):
         """Try to translate an ftplib exception to a bzrlib exception.

=== modified file 'bzrlib/transport/gio_transport.py'
--- a/bzrlib/transport/gio_transport.py	2010-05-28 18:52:03 +0000
+++ b/bzrlib/transport/gio_transport.py	2010-06-01 13:01:20 +0000
@@ -235,7 +235,13 @@
                                         " %s" % str(e), orig_error=e)
         return connection, (user, password)
 
+    def disconnect(self):
+        # FIXME: Nothing seems to be necessary here, which sounds a bit strange
+        # -- vila 20100601
+        pass
+
     def _reconnect(self):
+        # FIXME: This doesn't seem to be used -- vila 20100601
         """Create a new connection with the previously used credentials"""
         credentials = self._get_credentials()
         connection, credentials = self._create_connection(credentials)

=== modified file 'bzrlib/transport/http/__init__.py'
--- a/bzrlib/transport/http/__init__.py	2010-02-17 17:11:16 +0000
+++ b/bzrlib/transport/http/__init__.py	2010-06-01 13:01:20 +0000
@@ -629,6 +629,11 @@
         """
         pass
 
+    def disconnect(self):
+        """See SmartClientMedium.disconnect()."""
+        t = self._http_transport_ref()
+        t.disconnect()
+
 
 # TODO: May be better located in smart/medium.py with the other
 # SmartMediumRequest classes

=== modified file 'bzrlib/transport/http/_pycurl.py'
--- a/bzrlib/transport/http/_pycurl.py	2010-05-31 21:31:18 +0000
+++ b/bzrlib/transport/http/_pycurl.py	2010-06-01 13:01:20 +0000
@@ -128,6 +128,11 @@
             self._set_connection(connection, auth)
         return connection
 
+    def disconnect(self):
+        connection = self._get_connection()
+        if connection is not None:
+            connection.close()
+
     def has(self, relpath):
         """See Transport.has()"""
         # We set NO BODY=0 in _get_full, so it should be safe

=== modified file 'bzrlib/transport/http/_urllib.py'
--- a/bzrlib/transport/http/_urllib.py	2010-02-17 17:11:16 +0000
+++ b/bzrlib/transport/http/_urllib.py	2010-06-01 13:01:20 +0000
@@ -99,6 +99,11 @@
 
         return response
 
+    def disconnect(self):
+        connection = self._get_connection()
+        if connection is not None:
+            connection.close()
+
     def _get(self, relpath, offsets, tail_amount=0):
         """See HttpTransport._get"""
         abspath = self._remote_path(relpath)

=== modified file 'bzrlib/transport/remote.py'
--- a/bzrlib/transport/remote.py	2010-02-09 20:28:26 +0000
+++ b/bzrlib/transport/remote.py	2010-06-01 13:01:20 +0000
@@ -1,4 +1,4 @@
-# Copyright (C) 2006 Canonical Ltd
+# Copyright (C) 2006-2010 Canonical Ltd
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -435,7 +435,9 @@
         remote._translate_error(err, path=relpath)
 
     def disconnect(self):
-        self.get_smart_medium().disconnect()
+        m = self.get_smart_medium()
+        if m is not None:
+            m.disconnect()
 
     def stat(self, relpath):
         resp = self._call2('stat', self._remote_path(relpath))

=== modified file 'bzrlib/transport/sftp.py'
--- a/bzrlib/transport/sftp.py	2010-05-24 01:05:14 +0000
+++ b/bzrlib/transport/sftp.py	2010-06-01 13:01:20 +0000
@@ -389,6 +389,11 @@
                                          self._host, self._port)
         return connection, (user, password)
 
+    def disconnect(self):
+        connection = self._get_connection()
+        if connection is not None:
+            connection.close()
+
     def _get_sftp(self):
         """Ensures that a connection is established"""
         connection = self._get_connection()



More information about the bazaar-commits mailing list