diff options
| -rw-r--r-- | dev-requirements.txt | 4 | ||||
| -rw-r--r-- | paramiko/buffered_pipe.py | 19 | ||||
| -rw-r--r-- | sites/www/changelog.rst | 7 | ||||
| -rw-r--r-- | tasks.py | 2 | ||||
| -rw-r--r-- | tests/test_transport.py | 18 |
5 files changed, 42 insertions, 8 deletions
diff --git a/dev-requirements.txt b/dev-requirements.txt index 0c1fc020..2547fb5f 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -1,8 +1,8 @@ # Older junk tox>=1.4,<1.5 # For newer tasks like building Sphinx docs. -invoke>=0.11.1,<2.0 -invocations>=0.11.0,<2.0 +invoke>=0.13,<2.0 +invocations>=0.13,<2.0 sphinx>=1.1.3,<2.0 alabaster>=0.7.5,<2.0 releases>=1.1.0,<2.0 diff --git a/paramiko/buffered_pipe.py b/paramiko/buffered_pipe.py index d5fe164e..605f51e9 100644 --- a/paramiko/buffered_pipe.py +++ b/paramiko/buffered_pipe.py @@ -70,11 +70,20 @@ class BufferedPipe (object): :param threading.Event event: the event to set/clear """ - self._event = event - if len(self._buffer) > 0: - event.set() - else: - event.clear() + self._lock.acquire() + try: + self._event = event + # Make sure the event starts in `set` state if we appear to already + # be closed; otherwise, if we start in `clear` state & are closed, + # nothing will ever call `.feed` and the event (& OS pipe, if we're + # wrapping one - see `Channel.fileno`) will permanently stay in + # `clear`, causing deadlock if e.g. `select`ed upon. + if self._closed or len(self._buffer) > 0: + event.set() + else: + event.clear() + finally: + self._lock.release() def feed(self, data): """ diff --git a/sites/www/changelog.rst b/sites/www/changelog.rst index c5aeb48e..df818b47 100644 --- a/sites/www/changelog.rst +++ b/sites/www/changelog.rst @@ -2,6 +2,13 @@ Changelog ========= +* :bug:`537` Fix a bug in `BufferedPipe.set_event + <paramiko.buffered_pipe.BufferedPipe.set_event>` which could cause + deadlocks/hangs when one uses `select.select` against + `~paramiko.channel.Channel` objects (or otherwise calls `Channel.fileno + <paramiko.channel.Channel.fileno>` after the channel has closed). Thanks to + Przemysław Strzelczak for the report & reproduction case, and to Krzysztof + Rusek for the fix. * :release:`2.0.0 <2016-04-28>` * :release:`1.17.0 <2016-04-28>` * :release:`1.16.1 <2016-04-28>` @@ -2,7 +2,7 @@ from os import mkdir from os.path import join from shutil import rmtree, copytree -from invoke import Collection, ctask as task +from invoke import Collection, task from invocations.docs import docs, www, sites from invocations.packaging import publish diff --git a/tests/test_transport.py b/tests/test_transport.py index 5069e5b0..d81ad8f3 100644 --- a/tests/test_transport.py +++ b/tests/test_transport.py @@ -828,3 +828,21 @@ class TransportTest(unittest.TestCase): hostkey=public_host_key, username='slowdive', password='pygmalion') + + def test_M_select_after_close(self): + """ + verify that select works when a channel is already closed. + """ + self.setup_test_server() + chan = self.tc.open_session() + chan.invoke_shell() + schan = self.ts.accept(1.0) + schan.close() + + # give client a moment to receive close notification + time.sleep(0.1) + + r, w, e = select.select([chan], [], [], 0.1) + self.assertEqual([chan], r) + self.assertEqual([], w) + self.assertEqual([], e) |