launchpad-dev team mailing list archive

[Vincent Ladeuil <v.ladeuil+lp@xxxxxxx>]

>>>>> Jonathan Lange <jml@xxxxxxxxxxxxx> writes:

<snip/>
    >> 
    >> So it looks like the HttpServer instance needs to be killed in the test
    >> or in the teardown? I'm at a little bit of a loss, personally, so
    >> thought I'd throw it out there first.
    >> 

    > This seems a lot like https://bugs.edge.launchpad.net/bzr/+bug/193253,
    > although there it's a socket leaking check rather than a thread
    > leaking check. I don't know what's caused it to regress.

    > Specifically, there's code hidden by bzrlib that isn't cleaning up
    > after itself. Whether it should or not is an open question. From one
    > point of view, our thread checker is being overzealous, catching a
    > leak in something that's never going to affect production. From
    > another point of view, HttpServer.stop_server() should darn well stop
    > the server.

Yes it should.

    > Anyway, fixes are:
    >   * Fix bzrlib.tests.http_server to clean up its thread in stop_server


>From the bzr pov, it hasn't regressed :) Most thread/socket leaks have
even been fixed. But part of the fix was hackish (or was defeated
depending on the pov) and didn't properly capture all connections. These
connections are now cleaned up properly and they leak.

That's the first time this occurs on Ubuntu though, I knew about
occurrences on windows and OSX but all Ubuntu variants I've tested
didn't exhibit the problem (which is why it went unnoticed at first, or
rather, I thought it was windows specific).


   modified      bzrlib/tests/__init__.py
   modified      bzrlib/transport/__init__.py
                                                                        

=== modified file 'bzrlib/tests/__init__.py'
--- bzrlib/tests/__init__.py	2010-09-27 19:31:45 +0000
+++ bzrlib/tests/__init__.py	2010-10-07 08:09:18 +0000
@@ -2430,7 +2430,7 @@
                 self.addCleanup(t.disconnect)
             return t
 
-        orig_get_transport = self.overrideAttr(_mod_transport, 'get_transport',
+        orig_get_transport = self.overrideAttr(_mod_transport, '_get_transport',
                                                get_transport_with_cleanup)
         self._make_test_root()
         self.addCleanup(os.chdir, os.getcwdu())

=== modified file 'bzrlib/transport/__init__.py'
--- bzrlib/transport/__init__.py	2010-06-18 15:38:20 +0000
+++ bzrlib/transport/__init__.py	2010-10-07 08:09:18 +0000
@@ -1565,6 +1565,9 @@
 
 
 def get_transport(base, possible_transports=None):
+    return _get_transport(base, possible_transports)
+
+def _get_transport(base, possible_transports=None):
     """Open a transport to access a URL or directory.


The above is a hack above the hack but it should fix the problem until a
proper fix lands.

        Vincent