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