From d1b7c232bf76096b2f20bd86a4446b0e0ba164fd Mon Sep 17 00:00:00 2001 From: David Wilson Date: Sun, 4 Nov 2018 18:56:54 +0000 Subject: [PATCH 01/10] tests: image_prep needs sudo --- tests/image_prep/_container_setup.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/image_prep/_container_setup.yml b/tests/image_prep/_container_setup.yml index db0d3789..f62c5955 100644 --- a/tests/image_prep/_container_setup.yml +++ b/tests/image_prep/_container_setup.yml @@ -2,6 +2,7 @@ - hosts: all strategy: linear gather_facts: false + become: true tasks: - raw: > if ! python -c ''; then From 3836c6a22023011fa6028aeef6607cab4f7de3f4 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Sun, 4 Nov 2018 19:43:52 +0000 Subject: [PATCH 02/10] tests/bench: run roundtrip.py a ton more to reduce variance --- tests/bench/roundtrip.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/tests/bench/roundtrip.py b/tests/bench/roundtrip.py index 7c5a9252..33b3c5b8 100644 --- a/tests/bench/roundtrip.py +++ b/tests/bench/roundtrip.py @@ -5,13 +5,22 @@ Measure latency of local RPC. import mitogen import time +import ansible_mitogen.process +ansible_mitogen.process.setup_gil() + +try: + xrange +except NameError: + xrange = range + def do_nothing(): pass @mitogen.main() def main(router): f = router.fork() + f.call(do_nothing) t0 = time.time() - for x in range(1000): + for x in xrange(20000): f.call(do_nothing) - print '++', int(1e6 * ((time.time() - t0) / (1.0+x))), 'usec' + print('++', int(1e6 * ((time.time() - t0) / (1.0+x))), 'usec') From 6e1f9e25960219c94fc6c001d8eed624b456fd3f Mon Sep 17 00:00:00 2001 From: David Wilson Date: Sun, 4 Nov 2018 19:47:25 +0000 Subject: [PATCH 03/10] core: 2.6 str.decode() compat fix. --- mitogen/core.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mitogen/core.py b/mitogen/core.py index 58289553..370fb742 100644 --- a/mitogen/core.py +++ b/mitogen/core.py @@ -648,7 +648,7 @@ class Message(object): def _throw_dead(self): if len(self.data): - raise ChannelError(self.data.decode(errors='replace')) + raise ChannelError(self.data.decode('utf-8', 'replace')) elif self.src_id == mitogen.context_id: raise ChannelError(ChannelError.local_msg) else: From 5482b4d528a7e79e0291b27f5db40e5f1954d060 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Sun, 4 Nov 2018 19:48:42 +0000 Subject: [PATCH 04/10] tests: poller_test 3.x fix. --- tests/poller_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/poller_test.py b/tests/poller_test.py index a6190821..c214f367 100644 --- a/tests/poller_test.py +++ b/tests/poller_test.py @@ -43,7 +43,7 @@ class SockMixin(object): """Make `fd` unwriteable.""" while True: try: - os.write(fd, 'x'*4096) + os.write(fd, mitogen.core.b('x')*4096) except OSError: e = sys.exc_info()[1] if e.args[0] == errno.EAGAIN: From 27a4001f4f5b5e682ac103ce6069515bd6a4125d Mon Sep 17 00:00:00 2001 From: David Wilson Date: Sun, 4 Nov 2018 19:49:17 +0000 Subject: [PATCH 05/10] tests: handle NameError when faulthandler is not installed. --- tests/testlib.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/testlib.py b/tests/testlib.py index 2f3c2b2e..1406bf2d 100644 --- a/tests/testlib.py +++ b/tests/testlib.py @@ -20,7 +20,7 @@ import mitogen.utils try: import faulthandler except ImportError: - pass + faulthandler = None try: import urlparse From 3f46c9569c260a972c4790e2510510b9d9d898c5 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Sun, 4 Nov 2018 20:16:44 +0000 Subject: [PATCH 06/10] tests: 3.x syntax compat for tests/data/stubs/ --- tests/data/stubs/stub-lxc-info.py | 2 +- tests/data/stubs/stub-lxc.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/data/stubs/stub-lxc-info.py b/tests/data/stubs/stub-lxc-info.py index bcbc36a5..480bf266 100755 --- a/tests/data/stubs/stub-lxc-info.py +++ b/tests/data/stubs/stub-lxc-info.py @@ -1,4 +1,4 @@ #!/usr/bin/env python # Mainly for use in stubconnections/kubectl.yml -print 'PID: 1' +print('PID: 1') diff --git a/tests/data/stubs/stub-lxc.py b/tests/data/stubs/stub-lxc.py index 03572cac..9d14090a 100755 --- a/tests/data/stubs/stub-lxc.py +++ b/tests/data/stubs/stub-lxc.py @@ -5,7 +5,7 @@ import os # setns.py fetching leader PID. if sys.argv[1] == 'info': - print 'Pid: 1' + print('Pid: 1') sys.exit(0) os.environ['ORIGINAL_ARGV'] = repr(sys.argv) From 045db6f6890e54540827ddebc8eecdd6a22e2413 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Sun, 4 Nov 2018 20:18:23 +0000 Subject: [PATCH 07/10] Fix iter_read() FD leaks on 3.x; closes #418. --- mitogen/doas.py | 17 +++++++++++------ mitogen/parent.py | 16 ++++++++++------ mitogen/ssh.py | 19 ++++++++++++------- mitogen/su.py | 19 ++++++++++++------- mitogen/sudo.py | 28 +++++++++++++++++----------- 5 files changed, 62 insertions(+), 37 deletions(-) diff --git a/mitogen/doas.py b/mitogen/doas.py index 09b2be9e..3a6f881d 100644 --- a/mitogen/doas.py +++ b/mitogen/doas.py @@ -80,12 +80,7 @@ class Stream(mitogen.parent.Stream): password_incorrect_msg = 'doas password is incorrect' password_required_msg = 'doas password is required' - def _connect_bootstrap(self): - it = mitogen.parent.iter_read( - fds=[self.receive_side.fd, self.diag_stream.receive_side.fd], - deadline=self.connect_deadline, - ) - + def _connect_input_loop(self, it): password_sent = False for buf in it: LOG.debug('%r: received %r', self, buf) @@ -106,3 +101,13 @@ class Stream(mitogen.parent.Stream): ) password_sent = True raise mitogen.core.StreamError('bootstrap failed') + + def _connect_bootstrap(self): + it = mitogen.parent.iter_read( + fds=[self.receive_side.fd, self.diag_stream.receive_side.fd], + deadline=self.connect_deadline, + ) + try: + self._connect_input_loop(it) + finally: + it.close() diff --git a/mitogen/parent.py b/mitogen/parent.py index 04f7784e..865d8fd2 100644 --- a/mitogen/parent.py +++ b/mitogen/parent.py @@ -532,12 +532,16 @@ def discard_until(fd, s, deadline): :raises mitogen.core.StreamError: Attempt to read past end of file. """ - for buf in iter_read([fd], deadline): - if IOLOG.level == logging.DEBUG: - for line in buf.splitlines(): - IOLOG.debug('discard_until: discarding %r', line) - if buf.endswith(s): - return + it = iter_read([fd], deadline) + try: + for buf in it: + if IOLOG.level == logging.DEBUG: + for line in buf.splitlines(): + IOLOG.debug('discard_until: discarding %r', line) + if buf.endswith(s): + return + finally: + it.close() # ensure Poller.close() is called. def _upgrade_broker(broker): diff --git a/mitogen/ssh.py b/mitogen/ssh.py index e3891f9c..106dfd56 100644 --- a/mitogen/ssh.py +++ b/mitogen/ssh.py @@ -264,13 +264,7 @@ class Stream(mitogen.parent.Stream): # with ours. raise HostKeyError(self.hostkey_config_msg) - def _connect_bootstrap(self): - fds = [self.receive_side.fd] - if self.diag_stream is not None: - fds.append(self.diag_stream.receive_side.fd) - - it = mitogen.parent.iter_read(fds=fds, deadline=self.connect_deadline) - + def _connect_input_loop(self, it): password_sent = False for buf, partial in filter_debug(self, it): LOG.debug('%r: received %r', self, buf) @@ -302,3 +296,14 @@ class Stream(mitogen.parent.Stream): password_sent = True raise mitogen.core.StreamError('bootstrap failed') + + def _connect_bootstrap(self): + fds = [self.receive_side.fd] + if self.diag_stream is not None: + fds.append(self.diag_stream.receive_side.fd) + + it = mitogen.parent.iter_read(fds=fds, deadline=self.connect_deadline) + try: + self._connect_input_loop(it) + finally: + it.close() diff --git a/mitogen/su.py b/mitogen/su.py index 9b0172c8..65357a3e 100644 --- a/mitogen/su.py +++ b/mitogen/su.py @@ -87,13 +87,7 @@ class Stream(mitogen.parent.Stream): password_incorrect_msg = 'su password is incorrect' password_required_msg = 'su password is required' - def _connect_bootstrap(self): - password_sent = False - it = mitogen.parent.iter_read( - fds=[self.receive_side.fd], - deadline=self.connect_deadline, - ) - + def _connect_input_loop(self, it): for buf in it: LOG.debug('%r: received %r', self, buf) if buf.endswith(self.EC0_MARKER): @@ -113,3 +107,14 @@ class Stream(mitogen.parent.Stream): ) password_sent = True raise mitogen.core.StreamError('bootstrap failed') + + def _connect_bootstrap(self): + password_sent = False + it = mitogen.parent.iter_read( + fds=[self.receive_side.fd], + deadline=self.connect_deadline, + ) + try: + self._connect_input_loop(it) + finally: + it.close() diff --git a/mitogen/sudo.py b/mitogen/sudo.py index b2eaabce..68b27fec 100644 --- a/mitogen/sudo.py +++ b/mitogen/sudo.py @@ -173,17 +173,7 @@ class Stream(mitogen.parent.Stream): password_incorrect_msg = 'sudo password is incorrect' password_required_msg = 'sudo password is required' - def _connect_bootstrap(self): - fds = [self.receive_side.fd] - if self.diag_stream is not None: - fds.append(self.diag_stream.receive_side.fd) - - password_sent = False - it = mitogen.parent.iter_read( - fds=fds, - deadline=self.connect_deadline, - ) - + def _connect_input_loop(self, it): for buf in it: LOG.debug('%r: received %r', self, buf) if buf.endswith(self.EC0_MARKER): @@ -199,3 +189,19 @@ class Stream(mitogen.parent.Stream): ) password_sent = True raise mitogen.core.StreamError('bootstrap failed') + + def _connect_bootstrap(self): + fds = [self.receive_side.fd] + if self.diag_stream is not None: + fds.append(self.diag_stream.receive_side.fd) + + password_sent = False + it = mitogen.parent.iter_read( + fds=fds, + deadline=self.connect_deadline, + ) + + try: + self._connect_input_loop(it) + finally: + it.close() From a7eca5b55e66a13825175800ede8a65b41d45c5d Mon Sep 17 00:00:00 2001 From: David Wilson Date: Sun, 4 Nov 2018 20:20:41 +0000 Subject: [PATCH 08/10] docs: update Changelog. --- docs/changelog.rst | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index 93c21370..bb1974a8 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -235,6 +235,11 @@ Core Library was leaked on every RPC, due to a list of strong references keeping alive any handler ever registered for disconnect notification. +* `#418 `_: the + :func:`mitogen.parent.iter_read` helper would leak poller FDs, because + execution of its :keyword:`finally` block was delayed on Python 3. Now + callers explicitly close the generator when finished. + * `16ca111e `_: handle OpenSSH 7.5 permission denied prompts when ``~/.ssh/config`` rewrites are present. From e180d310b5eb0151435fbea64a4e372f0bede128 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Sun, 4 Nov 2018 20:27:52 +0000 Subject: [PATCH 09/10] tests: fix fork_test compat on 3.x. --- tests/fork_test.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/fork_test.py b/tests/fork_test.py index dd214bd1..39f5352e 100644 --- a/tests/fork_test.py +++ b/tests/fork_test.py @@ -16,14 +16,14 @@ import plain_old_module def _find_ssl_linux(): s = testlib.subprocess__check_output(['ldd', _ssl.__file__]) - for line in s.splitlines(): + for line in s.decode().splitlines(): bits = line.split() if bits[0].startswith('libssl'): return bits[2] def _find_ssl_darwin(): s = testlib.subprocess__check_output(['otool', '-l', _ssl.__file__]) - for line in s.splitlines(): + for line in s.decode().splitlines(): bits = line.split() if bits[0] == 'name' and 'libssl' in bits[1]: return bits[1] From 6d5facec4c5728f71f345f6e4e7b6738ea66888b Mon Sep 17 00:00:00 2001 From: David Wilson Date: Sun, 4 Nov 2018 20:43:29 +0000 Subject: [PATCH 10/10] su/sudo: fallout from previous commits issue #418 and FD cleanup work. --- mitogen/su.py | 4 +++- mitogen/sudo.py | 10 ++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/mitogen/su.py b/mitogen/su.py index 65357a3e..9faee2d4 100644 --- a/mitogen/su.py +++ b/mitogen/su.py @@ -88,6 +88,8 @@ class Stream(mitogen.parent.Stream): password_required_msg = 'su password is required' def _connect_input_loop(self, it): + password_sent = False + for buf in it: LOG.debug('%r: received %r', self, buf) if buf.endswith(self.EC0_MARKER): @@ -106,10 +108,10 @@ class Stream(mitogen.parent.Stream): mitogen.core.to_text(self.password + '\n').encode('utf-8') ) password_sent = True + raise mitogen.core.StreamError('bootstrap failed') def _connect_bootstrap(self): - password_sent = False it = mitogen.parent.iter_read( fds=[self.receive_side.fd], deadline=self.connect_deadline, diff --git a/mitogen/sudo.py b/mitogen/sudo.py index 68b27fec..cf8a44d9 100644 --- a/mitogen/sudo.py +++ b/mitogen/sudo.py @@ -116,10 +116,6 @@ class Stream(mitogen.parent.Stream): create_child = staticmethod(mitogen.parent.hybrid_tty_create_child) child_is_immediate_subprocess = False - #: Once connected, points to the corresponding DiagLogStream, allowing it to - #: be disconnected at the same time this stream is being torn down. - tty_stream = None - sudo_path = 'sudo' username = 'root' password = None @@ -174,6 +170,8 @@ class Stream(mitogen.parent.Stream): password_required_msg = 'sudo password is required' def _connect_input_loop(self, it): + password_sent = False + for buf in it: LOG.debug('%r: received %r', self, buf) if buf.endswith(self.EC0_MARKER): @@ -184,10 +182,11 @@ class Stream(mitogen.parent.Stream): raise PasswordError(self.password_required_msg) if password_sent: raise PasswordError(self.password_incorrect_msg) - self.tty_stream.transmit_side.write( + self.diag_stream.transmit_side.write( mitogen.core.to_text(self.password + '\n').encode('utf-8') ) password_sent = True + raise mitogen.core.StreamError('bootstrap failed') def _connect_bootstrap(self): @@ -195,7 +194,6 @@ class Stream(mitogen.parent.Stream): if self.diag_stream is not None: fds.append(self.diag_stream.receive_side.fd) - password_sent = False it = mitogen.parent.iter_read( fds=fds, deadline=self.connect_deadline,