Rev 5405: (jameinel) Fix bug #626876 by properly waiting for the paramiko.Transport in file:///home/pqm/archives/thelove/bzr/%2Btrunk/

[Canonical.com Patch Queue Manager]

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

------------------------------------------------------------
revno: 5405 [merge]
revision-id: pqm at pqm.ubuntu.com-20100902212734-6q910gzpfezmk4do
parent: pqm at pqm.ubuntu.com-20100901180619-u0q01om0gjzxxy2i
parent: john at arbash-meinel.com-20100902191954-2u2kwurchcmmpl9m
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Thu 2010-09-02 22:27:34 +0100
message:
  (jameinel) Fix bug #626876 by properly waiting for the paramiko.Transport
   thread to finish its work. (John A Meinel)
modified:
  NEWS                           NEWS-20050323055033-4e00b5db738777ff
  bzrlib/tests/stub_sftp.py      stub_sftp.py-20051027032739-0e7ef4f7bab0e174
  bzrlib/tests/test_transport.py testtransport.py-20050718175618-e5cdb99f4555ddce
=== modified file 'NEWS'
--- a/NEWS	2010-09-01 18:06:19 +0000
+++ b/NEWS	2010-09-01 19:10:42 +0000
@@ -158,6 +158,9 @@
   to a symlink, now returns information about the symlink.
   (Martin Pool)
 
+* Wait for the SSH server to actually finish, rather than just waiting for
+  it to negotiate the key exchange. (John Arbash Meinel, #626876)
+
 Improvements
 ************
 

=== modified file 'bzrlib/tests/stub_sftp.py'
--- a/bzrlib/tests/stub_sftp.py	2010-07-01 15:14:35 +0000
+++ b/bzrlib/tests/stub_sftp.py	2010-09-02 19:19:54 +0000
@@ -344,22 +344,25 @@
     def setup(self):
         self.wrap_for_latency()
         tcs = self.server.test_case_server
-        ssh_server = paramiko.Transport(self.request)
-        ssh_server.add_server_key(tcs.get_host_key())
-        ssh_server.set_subsystem_handler('sftp', paramiko.SFTPServer,
-                                         StubSFTPServer, root=tcs._root,
-                                         home=tcs._server_homedir)
+        ptrans = paramiko.Transport(self.request)
+        self.paramiko_transport = ptrans
+        # Set it to a channel under 'bzr' so that we get debug info
+        ptrans.set_log_channel('bzr.paramiko.transport')
+        ptrans.add_server_key(tcs.get_host_key())
+        ptrans.set_subsystem_handler('sftp', paramiko.SFTPServer,
+                                     StubSFTPServer, root=tcs._root,
+                                     home=tcs._server_homedir)
         server = tcs._server_interface(tcs)
-        ssh_server.start_server(None, server)
-        # FIXME: Long story short:
-        # bt.test_transport.TestSSHConnections.test_bzr_connect_to_bzr_ssh
-        # fails if we wait less than 0.2 seconds... paramiko uses a lot of
-        # timeouts internally which probably mask a synchronisation
-        # problem. Note that this is the only test that requires this hack and
-        # the test may need to be fixed instead, but it's late and the test is
-        # horrible as mentioned in its comments :) -- vila 20100623
-        import time
-        time.sleep(0.2)
+        # This blocks until the key exchange has been done
+        ptrans.start_server(None, server)
+
+    def finish(self):
+        # Wait for the conversation to finish, when the paramiko.Transport
+        # thread finishes
+        # TODO: Consider timing out after XX seconds rather than hanging.
+        #       Also we could check paramiko_transport.active and possibly
+        #       paramiko_transport.getException().
+        self.paramiko_transport.join()
 
     def wrap_for_latency(self):
         tcs = self.server.test_case_server
@@ -380,7 +383,7 @@
             def get_transport(self):
                 return self
             def get_log_channel(self):
-                return 'paramiko'
+                return 'bzr.paramiko'
             def get_name(self):
                 return '1'
             def get_hexdump(self):
@@ -389,11 +392,13 @@
                 pass
 
         tcs = self.server.test_case_server
-        server = paramiko.SFTPServer(
+        sftp_server = paramiko.SFTPServer(
             FakeChannel(), 'sftp', StubServer(tcs), StubSFTPServer,
             root=tcs._root, home=tcs._server_homedir)
+        self.sftp_server = sftp_server
+        sys_stderr = sys.stderr # Used in error reporting during shutdown
         try:
-            server.start_subsystem(
+            sftp_server.start_subsystem(
                 'sftp', None, ssh.SocketAsChannelAdapter(self.request))
         except socket.error, e:
             if (len(e.args) > 0) and (e.args[0] == errno.EPIPE):
@@ -409,10 +414,11 @@
             # seems to be the best we can do.
             # FIXME: All interpreter shutdown errors should have been related
             # to daemon threads, cleanup needed -- vila 20100623
-            import sys
-            sys.stderr.write('\nEXCEPTION %r: ' % (e.__class__,))
-            sys.stderr.write('%s\n\n' % (e,))
-        server.finish_subsystem()
+            sys_stderr.write('\nEXCEPTION %r: ' % (e.__class__,))
+            sys_stderr.write('%s\n\n' % (e,))
+
+    def finish(self):
+        self.sftp_server.finish_subsystem()
 
 
 class TestingSFTPServer(test_server.TestingThreadingTCPServer):

=== modified file 'bzrlib/tests/test_transport.py'
--- a/bzrlib/tests/test_transport.py	2010-08-30 21:27:50 +0000
+++ b/bzrlib/tests/test_transport.py	2010-09-01 18:43:32 +0000
@@ -900,8 +900,6 @@
 
         bzr+ssh:// should cause bzr to run a remote bzr smart server over SSH.
         """
-        raise tests.TestSkipped('this test was recently broken,'
-                                ' see bug #626876')
         # This test actually causes a bzr instance to be invoked, which is very
         # expensive: it should be the only such test in the test suite.
         # A reasonable evolution for this would be to simply check inside