[project @ Arch-1:robey@lag.net--2005-master-shake%paramiko--dev--1--patch-34]

fix a comment claiming that channels are closed automatically when GC'd (they aren't and can't be); don't close the pipe until the app explicitly calls close(); signal EOF via the pipe

paramiko/channel.py

@@ -454,24 +454,25 @@ class Channel (object):
         """
         Close the channel.  All future read/write operations on the channel
         will fail.  The remote end will receive no more data (after queued data
-        is flushed).  Channels are automatically closed when they are garbage-
+        is flushed).  Channels are automatically closed when their L{Transport}
-        collected, or when their L{Transport} is closed.
+        is closed.
+        
+        Note that because of peculiarities with the way python's garbage
+        collection works on cycles, channels will B{not} be automatically
+        closed by the garbage collector.
         """
         self.lock.acquire()
         try:
             if not self.active or self.closed:
                 return
-            try:
+            self._close_internal()
-                self._send_eof()
+
-                m = Message()
+            # only close the pipe when the user explicitly closes the channel.
-                m.add_byte(chr(MSG_CHANNEL_CLOSE))
+            # otherwise they will get unpleasant surprises.
-                m.add_int(self.remote_chanid)
+            if self.pipe is not None:
-                self.transport._send_user_message(m)
+                print 'closing pipe!'
-            except EOFError:
+                self.pipe.close()
-                pass
+                self.pipe = None
-            self._set_closed()
-            # can't unlink from the Transport yet -- the remote side may still
-            # try to send meta-data (exit-status, etc)
         finally:
             self.lock.release()
 
@@ -862,7 +863,11 @@ class Channel (object):
         return
 
     def _request_failed(self, m):
-        self.close()
+        self.lock.acquire()
+        try:
+            self._close_internal()
+        finally:
+            self.lock.release()
 
     def _feed(self, m):
         if type(m) is str:
@@ -980,20 +985,16 @@ class Channel (object):
                 self.in_buffer_cv.notifyAll()
                 self.in_stderr_buffer_cv.notifyAll()
                 if self.pipe is not None:
-                    self.pipe.close()
+                    self.pipe.set()
-                    self.pipe = None
         finally:
             self.lock.release()
         self._log(DEBUG, 'EOF received')
 
     def _handle_close(self, m):
-        self.close()
         self.lock.acquire()
         try:
+            self._close_internal()
             self.transport._unlink_channel(self.chanid)
-            if self.pipe is not None:
-                self.pipe.close()
-                self.pipe = None
         finally:
             self.lock.release()
 
@@ -1023,6 +1024,22 @@ class Channel (object):
         self._log(DEBUG, 'EOF sent')
         return
 
+    def _close_internal(self):
+        # you are holding the lock.
+        if not self.active or self.closed:
+            return
+        try:
+            self._send_eof()
+            m = Message()
+            m.add_byte(chr(MSG_CHANNEL_CLOSE))
+            m.add_int(self.remote_chanid)
+            self.transport._send_user_message(m)
+        except EOFError:
+            pass
+        self._set_closed()
+        # can't unlink from the Transport yet -- the remote side may still
+        # try to send meta-data (exit-status, etc)
+
     def _unlink(self):
         # server connection could die before we become active: still signal the close!
         if self.closed: