From 068bf63cf03d689187bede5c11b6c38ec2b2a8e1 Mon Sep 17 00:00:00 2001 From: Frank Arnold Date: Thu, 11 Apr 2013 18:56:47 +0200 Subject: [PATCH 1/3] transport: Wait for thread termination before closing the socket Make sure the Thread.run() method has terminated before closing the socket. Currently, the socket is closed through Packetizer.close(), which happens too early. Move the socket.close() into Transport.close() and after the Thread.join() call. While at it, modify the stop_thread() method and use it in Transport.close() to avoid code duplication. Use join() with a timeout to make it possible to terminate the main thread with KeyboardInterrupt. Also, remove the now obsolete socket.close() from Transport.atfork(). This fixes a potential infinite loop if paramiko.SSHClient is connected through a paramiko.Channel instead of a regular socket (tunneling). Details: Using a debug patch to dump the current stack of the thread every couple of seconds while trying to close it, I've seen the following over and over again: Thread could not be stopped, still running. Current traceback (most recent call last): File "/usr/lib/python2.7/threading.py", line 524, in __bootstrap self.__bootstrap_inner() File "/usr/lib/python2.7/threading.py", line 551, in __bootstrap_inner self.run() File ".../paramiko/transport.py", line 1564, in run self._channel_handler_table[ptype](chan, m) File ".../paramiko/channel.py", line 1102, in _handle_close self.transport._send_user_message(m) File ".../paramiko/transport.py", line 1418, in _send_user_message self._send_message(data) File ".../paramiko/transport.py", line 1398, in _send_message self.packetizer.send_message(data) File ".../paramiko/packet.py", line 319, in send_message self.write_all(out) File ".../paramiko/packet.py", line 248, in write_all n = self.__socket.send(out) File ".../paramiko/channel.py", line 732, in send self.lock.release() The thread was running Packetizer.write_all() in an endless loop: while len(out) > 0: ... n = Channel.send(out) # n == 0 because channel got closed ... out = out[n:] # essentially out = out Signed-off-by: Frank Arnold --- paramiko/packet.py | 1 - paramiko/transport.py | 8 ++++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/paramiko/packet.py b/paramiko/packet.py index 38a6d4b..2887bde 100644 --- a/paramiko/packet.py +++ b/paramiko/packet.py @@ -156,7 +156,6 @@ class Packetizer (object): def close(self): self.__closed = True - self.__socket.close() def set_hexdump(self, hexdump): self.__dump_packets = hexdump diff --git a/paramiko/transport.py b/paramiko/transport.py index fd6dab7..0197ced 100644 --- a/paramiko/transport.py +++ b/paramiko/transport.py @@ -400,7 +400,6 @@ class Transport (threading.Thread): @since: 1.5.3 """ - self.sock.close() self.close() def get_security_options(self): @@ -614,11 +613,10 @@ class Transport (threading.Thread): """ if not self.active: return - self.active = False - self.packetizer.close() - self.join() + self.stop_thread() for chan in self._channels.values(): chan._unlink() + self.sock.close() def get_remote_server_key(self): """ @@ -1391,6 +1389,8 @@ class Transport (threading.Thread): def stop_thread(self): self.active = False self.packetizer.close() + while self.is_alive(): + self.join(10) ### internals... From 5c124cb1362f08296802f8d4856eaa18c0b35b00 Mon Sep 17 00:00:00 2001 From: Steven Noonan Date: Thu, 11 Apr 2013 16:27:49 -0700 Subject: [PATCH 2/3] un-break Python 2.5 compatibility by using isAlive() instead of is_alive() Python's documentation has a bug[1], in that it doesn't correctly annotate is_alive as being a function introduced in Python 2.6. [1] http://bugs.python.org/issue15126 Signed-off-by: Steven Noonan --- paramiko/transport.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/paramiko/transport.py b/paramiko/transport.py index 0197ced..fd63732 100644 --- a/paramiko/transport.py +++ b/paramiko/transport.py @@ -1389,7 +1389,7 @@ class Transport (threading.Thread): def stop_thread(self): self.active = False self.packetizer.close() - while self.is_alive(): + while self.isAlive(): self.join(10) From e1851788768b5132181690e5ab03d4d65c466e42 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Fri, 27 Sep 2013 17:33:53 -0700 Subject: [PATCH 3/3] Changelog re #156, closes #156 --- NEWS | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/NEWS b/NEWS index e3deb14..1649d5e 100644 --- a/NEWS +++ b/NEWS @@ -12,6 +12,13 @@ Issues noted as "Fabric #NN" can be found at https://github.com/fabric/fabric/. Releases ======== +v1.11.2 (27th Sep 2013) +----------------------- + +* #156: Fix potential deadlock condition when using Channel objects as sockets + (e.g. when using SSH gatewaying). Thanks to Steven Noonan and Frank Arnold + for catch & patch. + v1.10.4 (27th Sep 2013) -----------------------