From 48bb10694b67a66f2c524dcca8a867319e60b87c Mon Sep 17 00:00:00 2001 From: Robey Pointer Date: Mon, 20 Nov 2006 11:21:37 -0800 Subject: [PATCH] [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 | 34 ++++++++++++++++++---------------- paramiko/pipe.py | 6 ++++++ tests/test_transport.py | 5 +++++ 3 files changed, 29 insertions(+), 16 deletions(-) diff --git a/paramiko/channel.py b/paramiko/channel.py index 5c4d5e8..48b4487 100644 --- a/paramiko/channel.py +++ b/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: - return self.pipe.fileno() + if self._pipe is not None: + return self._pipe.fileno() # create the pipe and feed in any existing data - self.pipe = pipe.make_pipe() - self.in_buffer.set_event(self.pipe) - return self.pipe.fileno() + self._pipe = pipe.make_pipe() + self.in_buffer.set_event(self._pipe) + 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: - self.pipe.set_forever() + if self._pipe is not None: + 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: - self.pipe.set_forever() + if self._pipe is not None: + self._pipe.set_forever() def _send_eof(self): # you are holding the lock. diff --git a/paramiko/pipe.py b/paramiko/pipe.py index 5230099..0fa2bbf 100644 --- a/paramiko/pipe.py +++ b/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() diff --git a/tests/test_transport.py b/tests/test_transport.py index 60ac1e7..e3763ee 100644 --- a/tests/test_transport.py +++ b/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): """