[project @ robey@lag.net-20061120192137-1rcpiiq9mcd58m76]

reorder the closing of the pipe in Channel.close() to make sure it happens even when the channel is closed by the remote host first

paramiko/channel.py

@@ -86,7 +86,7 @@ class Channel (object):
         self.status_event = threading.Event()
         self.name = str(chanid)
         self.logger = util.get_logger('paramiko.chan.' + str(chanid))
-        self.pipe = None
+        self._pipe = None
         self.event = threading.Event()
         self.combine_stderr = False
         self.exit_status = -1
@@ -457,15 +457,17 @@ class Channel (object):
         """
         self.lock.acquire()
         try:
+            # only close the pipe when the user explicitly closes the channel.
+            # otherwise they will get unpleasant surprises.  (and do it before
+            # checking self.closed, since the remote host may have already
+            # closed the connection.)
+            if self._pipe is not None:
+                self._pipe.close()
+                self._pipe = None
+
             if not self.active or self.closed:
                 return
             msgs = self._close_internal()
-
-            # only close the pipe when the user explicitly closes the channel.
-            # otherwise they will get unpleasant surprises.
-            if self.pipe is not None:
-                self.pipe.close()
-                self.pipe = None
         finally:
             self.lock.release()
         for m in msgs:
@@ -732,12 +734,12 @@ class Channel (object):
         """
         self.lock.acquire()
         try:
-            if self.pipe is not None:
+            if self._pipe is not None:
-                return self.pipe.fileno()
+                return self._pipe.fileno()
             # create the pipe and feed in any existing data
-            self.pipe = pipe.make_pipe()
+            self._pipe = pipe.make_pipe()
-            self.in_buffer.set_event(self.pipe)
+            self.in_buffer.set_event(self._pipe)
-            return self.pipe.fileno()
+            return self._pipe.fileno()
         finally:
             self.lock.release()
 
@@ -926,8 +928,8 @@ class Channel (object):
                 self.eof_received = True
                 self.in_buffer.close()
                 self.in_stderr_buffer.close()
-                if self.pipe is not None:
+                if self._pipe is not None:
-                    self.pipe.set_forever()
+                    self._pipe.set_forever()
         finally:
             self.lock.release()
         self._log(DEBUG, 'EOF received')
@@ -968,8 +970,8 @@ class Channel (object):
         self.in_buffer.close()
         self.in_stderr_buffer.close()
         self.out_buffer_cv.notifyAll()
-        if self.pipe is not None:
+        if self._pipe is not None:
-            self.pipe.set_forever()
+            self._pipe.set_forever()
 
     def _send_eof(self):
         # you are holding the lock.

paramiko/pipe.py

@@ -39,10 +39,13 @@ class PosixPipe (object):
         self._rfd, self._wfd = os.pipe()
         self._set = False
         self._forever = False
+        self._closed = False
     
     def close (self):
         os.close(self._rfd)
         os.close(self._wfd)
+        # used for unit tests:
+        self._closed = True
     
     def fileno (self):
         return self._rfd
@@ -82,10 +85,13 @@ class WindowsPipe (object):
         serv.close()
         self._set = False
         self._forever = False
+        self._closed = False
     
     def close (self):
         self._rsock.close()
         self._wsock.close()
+        # used for unit tests:
+        self._closed = True
     
     def fileno (self):
         return self._rsock.fileno()

tests/test_transport.py

@@ -538,7 +538,12 @@ class TransportTest (unittest.TestCase):
         self.assertEquals([], e)
         self.assertEquals('', chan.recv(16))
         
+        # make sure the pipe is still open for now...
+        p = chan._pipe
+        self.assertEquals(False, p._closed)
         chan.close()
+        # ...and now is closed.
+        self.assertEquals(True, p._closed)
    
     def test_G_renegotiate(self):
         """