From 68d6e6bf34d26f9989c6d6b162371a4c67a3e53b Mon Sep 17 00:00:00 2001 From: David Wilson Date: Tue, 8 May 2018 18:30:44 +0100 Subject: [PATCH 01/59] ansible: tests for all SSH variables. Closes #184. --- tests/ansible/integration/ssh/all.yml | 1 + tests/ansible/integration/ssh/variables.yml | 101 ++++++++++++++++++++ tests/ansible/osx_setup.yml | 24 +++++ 3 files changed, 126 insertions(+) create mode 100644 tests/ansible/integration/ssh/variables.yml diff --git a/tests/ansible/integration/ssh/all.yml b/tests/ansible/integration/ssh/all.yml index 8a3b7f88..2425943a 100644 --- a/tests/ansible/integration/ssh/all.yml +++ b/tests/ansible/integration/ssh/all.yml @@ -1 +1,2 @@ - import_playbook: timeouts.yml +- import_playbook: variables.yml diff --git a/tests/ansible/integration/ssh/variables.yml b/tests/ansible/integration/ssh/variables.yml new file mode 100644 index 00000000..e9fa3584 --- /dev/null +++ b/tests/ansible/integration/ssh/variables.yml @@ -0,0 +1,101 @@ + +- name: integration/ssh/variables.yml + hosts: test-targets + connection: local + vars: + # ControlMaster has the effect of caching the previous auth to the same + # account, so disable it. Can't disable with ControlMaster no since that + # already appears on command line, so override ControlPath with junk. + ansible_ssh_common_args: | + -o "ControlPath /tmp/mitogen-ansible-test-{{18446744073709551615|random}}" + + tasks: + - name: ansible_ssh_user + # Remaining tests just use "ansible_user". + shell: > + ANSIBLE_STRATEGY=mitogen_linear + ANSIBLE_SSH_ARGS="" + ansible -m shell -a whoami -i "{{inventory_file}}" test-targets + -e ansible_ssh_user=mitogen__has_sudo + -e ansible_ssh_pass=has_sudo_password + register: out + + - shell: > + ANSIBLE_STRATEGY=mitogen_linear + ANSIBLE_SSH_ARGS="" + ansible -m shell -a whoami -i "{{inventory_file}}" test-targets + -e ansible_ssh_user=mitogen__has_sudo + -e ansible_ssh_pass=wrong_password + register: out + ignore_errors: true + + - assert: + that: out.rc == 4 # unreachable + + + - name: ansible_ssh_pass + shell: > + ANSIBLE_STRATEGY=mitogen_linear + ANSIBLE_SSH_ARGS="" + ansible -m shell -a whoami -i "{{inventory_file}}" test-targets + -e ansible_user=mitogen__has_sudo + -e ansible_ssh_pass=has_sudo_password + register: out + + - shell: > + ANSIBLE_STRATEGY=mitogen_linear + ANSIBLE_SSH_ARGS="" + ansible -m shell -a whoami -i "{{inventory_file}}" test-targets + -e ansible_user=mitogen__has_sudo + -e ansible_ssh_pass=wrong_password + register: out + ignore_errors: true + + - assert: + that: out.rc == 4 # unreachable + + + - name: ansible_password + shell: > + ANSIBLE_STRATEGY=mitogen_linear + ANSIBLE_SSH_ARGS="" + ansible -m shell -a whoami -i "{{inventory_file}}" test-targets + -e ansible_user=mitogen__has_sudo + -e ansible_password=has_sudo_password + register: out + + - shell: > + ANSIBLE_STRATEGY=mitogen_linear + ANSIBLE_SSH_ARGS="" + ansible -m shell -a whoami -i "{{inventory_file}}" test-targets + -e ansible_user=mitogen__has_sudo + -e ansible_password=wrong_password + register: out + ignore_errors: true + + - assert: + that: out.rc == 4 # unreachable + + + - name: ansible_ssh_private_key_file + shell: > + ANSIBLE_STRATEGY=mitogen_linear + ANSIBLE_SSH_ARGS="" + ansible -m shell -a whoami -i "{{inventory_file}}" test-targets + -e ansible_user=mitogen__has_sudo_pubkey + -e ansible_ssh_private_key_file=../data/docker/mitogen__has_sudo_pubkey.key + register: out + + - shell: > + ANSIBLE_STRATEGY=mitogen_linear + ANSIBLE_SSH_ARGS="" + ansible -m shell -a whoami -i "{{inventory_file}}" test-targets + -e ansible_user=mitogen__has_sudo + -e ansible_ssh_private_key_file=/dev/null + register: out + ignore_errors: true + + - assert: + that: out.rc == 4 # unreachable + + diff --git a/tests/ansible/osx_setup.yml b/tests/ansible/osx_setup.yml index 3c53fd8f..655c7605 100644 --- a/tests/ansible/osx_setup.yml +++ b/tests/ansible/osx_setup.yml @@ -20,12 +20,19 @@ # # Hashed passwords. # + - name: Create Mitogen test group + group: + name: "mitogen__group" + - name: Create Mitogen test users user: name: "mitogen__{{item}}" shell: /bin/bash + groups: mitogen__group password: "{{ (item + '_password') | password_hash('sha256') }}" with_items: + - has_sudo + - has_sudo_pubkey - require_tty - pw_required - require_tty_pw_required @@ -47,8 +54,11 @@ user: name: "mitogen__{{item}}" shell: /bin/bash + groups: mitogen__group password: "{{item}}_password" with_items: + - has_sudo + - has_sudo_pubkey - require_tty - pw_required - require_tty_pw_required @@ -98,6 +108,20 @@ - bashrc - profile + - name: Install pubkey for one account + file: + path: ~mitogen__has_sudo_pubkey/.ssh + state: directory + mode: go= + owner: mitogen__has_sudo_pubkey + + - name: Install pubkey for one account + copy: + dest: ~mitogen__has_sudo_pubkey/.ssh/authorized_keys + src: ../data/docker/mitogen__has_sudo_pubkey.key.pub + mode: go= + owner: mitogen__has_sudo_pubkey + - name: Require a TTY for two accounts lineinfile: path: /etc/sudoers From faaac43a78384b79b790c18e98a7053fea6729aa Mon Sep 17 00:00:00 2001 From: David Wilson Date: Tue, 8 May 2018 19:26:43 +0100 Subject: [PATCH 02/59] Disable SSH variables test on vanilla Ansible. --- tests/ansible/integration/ssh/variables.yml | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/tests/ansible/integration/ssh/variables.yml b/tests/ansible/integration/ssh/variables.yml index e9fa3584..fd1a610b 100644 --- a/tests/ansible/integration/ssh/variables.yml +++ b/tests/ansible/integration/ssh/variables.yml @@ -1,4 +1,7 @@ +# These tests don't run on vanilla because ssh-askpass wants to run for +# whatever reason. + - name: integration/ssh/variables.yml hosts: test-targets connection: local @@ -19,6 +22,7 @@ -e ansible_ssh_user=mitogen__has_sudo -e ansible_ssh_pass=has_sudo_password register: out + when: is_mitogen - shell: > ANSIBLE_STRATEGY=mitogen_linear @@ -28,9 +32,11 @@ -e ansible_ssh_pass=wrong_password register: out ignore_errors: true + when: is_mitogen - assert: that: out.rc == 4 # unreachable + when: is_mitogen - name: ansible_ssh_pass @@ -41,6 +47,7 @@ -e ansible_user=mitogen__has_sudo -e ansible_ssh_pass=has_sudo_password register: out + when: is_mitogen - shell: > ANSIBLE_STRATEGY=mitogen_linear @@ -50,9 +57,11 @@ -e ansible_ssh_pass=wrong_password register: out ignore_errors: true + when: is_mitogen - assert: that: out.rc == 4 # unreachable + when: is_mitogen - name: ansible_password @@ -63,6 +72,7 @@ -e ansible_user=mitogen__has_sudo -e ansible_password=has_sudo_password register: out + when: is_mitogen - shell: > ANSIBLE_STRATEGY=mitogen_linear @@ -72,9 +82,11 @@ -e ansible_password=wrong_password register: out ignore_errors: true + when: is_mitogen - assert: that: out.rc == 4 # unreachable + when: is_mitogen - name: ansible_ssh_private_key_file @@ -85,6 +97,7 @@ -e ansible_user=mitogen__has_sudo_pubkey -e ansible_ssh_private_key_file=../data/docker/mitogen__has_sudo_pubkey.key register: out + when: is_mitogen - shell: > ANSIBLE_STRATEGY=mitogen_linear @@ -94,8 +107,8 @@ -e ansible_ssh_private_key_file=/dev/null register: out ignore_errors: true + when: is_mitogen - assert: that: out.rc == 4 # unreachable - - + when: is_mitogen From 78d375e0c5dfe5ae029c5266646788baa263c967 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Fri, 11 May 2018 14:56:20 +0100 Subject: [PATCH 03/59] core: CallError should handle any exception type. Previously SystemExit would be pickled. --- mitogen/core.py | 7 ++++++- tests/call_error_test.py | 5 +++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/mitogen/core.py b/mitogen/core.py index 6e3ad3bb..26151cbf 100644 --- a/mitogen/core.py +++ b/mitogen/core.py @@ -78,6 +78,11 @@ LOAD_MODULE = 107 DETACHING = 108 IS_DEAD = 999 +try: + BaseException +except NameError: + BaseException = Exception + PY3 = sys.version_info > (3,) if PY3: b = lambda s: s.encode('latin-1') @@ -134,7 +139,7 @@ class Secret(UnicodeType): class CallError(Error): def __init__(self, fmt=None, *args): - if not isinstance(fmt, Exception): + if not isinstance(fmt, BaseException): Error.__init__(self, fmt, *args) else: e = fmt diff --git a/tests/call_error_test.py b/tests/call_error_test.py index f96aae27..ab9a2660 100644 --- a/tests/call_error_test.py +++ b/tests/call_error_test.py @@ -22,6 +22,11 @@ class ConstructorTest(unittest2.TestCase): e = self.klass(ve) self.assertEquals(e[0], 'exceptions.ValueError: eek') + def test_form_base_exc(self): + ve = SystemExit('eek') + e = self.klass(ve) + self.assertEquals(e[0], 'exceptions.SystemExit: eek') + def test_from_exc_tb(self): try: raise ValueError('eek') From 6a170420201beed7e7a99039afaca8128e8b9fd8 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Sat, 12 May 2018 14:09:54 +0100 Subject: [PATCH 04/59] issue #217: ansible: merge duplicate ReplacerPlanner. --- ansible_mitogen/planner.py | 53 +++++++++++++++++--------------------- 1 file changed, 23 insertions(+), 30 deletions(-) diff --git a/ansible_mitogen/planner.py b/ansible_mitogen/planner.py index f007205c..f19d77d4 100644 --- a/ansible_mitogen/planner.py +++ b/ansible_mitogen/planner.py @@ -228,36 +228,6 @@ class ScriptPlanner(BinaryPlanner): ) -class ReplacerPlanner(BinaryPlanner): - """ - The Module Replacer framework is the original framework implementing - new-style modules. It is essentially a preprocessor (like the C - Preprocessor for those familiar with that programming language). It does - straight substitutions of specific substring patterns in the module file. - There are two types of substitutions. - - * Replacements that only happen in the module file. These are public - replacement strings that modules can utilize to get helpful boilerplate - or access to arguments. - - "from ansible.module_utils.MOD_LIB_NAME import *" is replaced with the - contents of the ansible/module_utils/MOD_LIB_NAME.py. These should only - be used with new-style Python modules. - - "#<>" is equivalent to - "from ansible.module_utils.basic import *" and should also only apply to - new-style Python modules. - - "# POWERSHELL_COMMON" substitutes the contents of - "ansible/module_utils/powershell.ps1". It should only be used with - new-style Powershell modules. - """ - runner_name = 'ReplacerRunner' - - def detect(self, invocation): - return module_common.REPLACER in invocation.module_source - - class JsonArgsPlanner(ScriptPlanner): """ Script that has its interpreter directive and the task arguments @@ -313,6 +283,29 @@ class NewStylePlanner(ScriptPlanner): class ReplacerPlanner(NewStylePlanner): + """ + The Module Replacer framework is the original framework implementing + new-style modules. It is essentially a preprocessor (like the C + Preprocessor for those familiar with that programming language). It does + straight substitutions of specific substring patterns in the module file. + There are two types of substitutions. + + * Replacements that only happen in the module file. These are public + replacement strings that modules can utilize to get helpful boilerplate + or access to arguments. + + "from ansible.module_utils.MOD_LIB_NAME import *" is replaced with the + contents of the ansible/module_utils/MOD_LIB_NAME.py. These should only + be used with new-style Python modules. + + "#<>" is equivalent to + "from ansible.module_utils.basic import *" and should also only apply to + new-style Python modules. + + "# POWERSHELL_COMMON" substitutes the contents of + "ansible/module_utils/powershell.ps1". It should only be used with + new-style Powershell modules. + """ runner_name = 'ReplacerRunner' def detect(self, invocation): From 0f8190eff6f17299ae29098b01eb36cb7a4a2305 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Sat, 12 May 2018 14:10:19 +0100 Subject: [PATCH 05/59] issue #217: ansible: working module_finder.py --- ansible_mitogen/module_finder.py | 135 ++++++++++++------------------- 1 file changed, 52 insertions(+), 83 deletions(-) diff --git a/ansible_mitogen/module_finder.py b/ansible_mitogen/module_finder.py index 1b2e1850..dd20237d 100644 --- a/ansible_mitogen/module_finder.py +++ b/ansible_mitogen/module_finder.py @@ -1,49 +1,30 @@ +from __future__ import absolute_import import imp import os import sys -import mitogen.master - - -class Name(object): - def __str__(self): - return self.identifier - - def __repr__(self): - return 'Name(%r)' % (self.identifier,) - def __init__(self, identifier): - self.identifier = identifier - - def head(self): - head, _, tail = self.identifier.partition('.') - return head - - def tail(self): - head, _, tail = self.identifier.partition('.') - return tail +import mitogen.master +import ansible.module_utils - def pop_n(self, level): - name = self.identifier - for _ in xrange(level): - if '.' not in name: - return None - name, _, _ = self.identifier.rpartition('.') - return Name(name) - def append(self, part): - return Name('%s.%s' % (self.identifier, part)) +PREFIX = 'ansible.module_utils.' class Module(object): def __init__(self, name, path, kind=imp.PY_SOURCE, parent=None): - self.name = Name(name) + self.name = name self.path = path - if kind == imp.PKG_DIRECTORY: - self.path = os.path.join(self.path, '__init__.py') self.kind = kind + self.is_pkg = kind == imp.PKG_DIRECTORY self.parent = parent + def __hash__(self): + return hash(self.path) + + def __eq__(self, other): + return self.path == other.path + def fullname(self): bits = [str(self.name)] while self.parent: @@ -51,16 +32,6 @@ class Module(object): self = self.parent return '.'.join(reversed(bits)) - def __repr__(self): - return 'Module(%r, path=%r, parent=%r)' % ( - self.name, - self.path, - self.parent, - ) - - def dirname(self): - return os.path.dirname(self.path) - def code(self): fp = open(self.path) try: @@ -73,8 +44,9 @@ def find(name, path=(), parent=None): """ (Name, search path) -> Module instance or None. """ + head, _, tail = name.partition('.') try: - tup = imp.find_module(name.head(), list(path)) + tup = imp.find_module(head, list(path)) except ImportError: return parent @@ -82,56 +54,53 @@ def find(name, path=(), parent=None): if fp: fp.close() - module = Module(name.head(), path, kind, parent) - if name.tail(): - return find_relative(module, Name(name.tail()), path) + if kind == imp.PKG_DIRECTORY: + path = os.path.join(path, '__init__.py') + module = Module(head, path, kind, parent) + if tail: + return find_relative(module, tail, path) return module def find_relative(parent, name, path=()): - path = [parent.dirname()] + list(path) + path = [os.path.dirname(parent.path)] + list(path) return find(name, path, parent=parent) -def path_pop(s, n): - return os.pathsep.join(s.split(os.pathsep)[-n:]) - - -def scan(module, path): - scanner = mitogen.master.scan_code_imports(module.code()) - for level, modname_s, fromlist in scanner: - modname = Name(modname_s) - if level == -1: - imported = find_relative(module, modname, path) - elif level: - subpath = [path_pop(module.dirname(), level)] + list(path) - imported = find(modname.pop_n(level), subpath) - else: - imported = find(modname.pop_n(level), path) - - if imported and mitogen.master.is_stdlib_path(imported.path): - continue - - if imported and fromlist: - have = False - for fromname_s in fromlist: - fromname = modname.append(fromname_s) - f_imported = find_relative(imported, fromname, path) - if f_imported and f_imported.fullname() == fromname.identifier: - have = True - yield fromname, f_imported, None - if have: - continue +def scan_fromlist(code): + for level, modname_s, fromlist in mitogen.master.scan_code_imports(code): + for name in fromlist: + yield level, '%s.%s' % (modname_s, name) + if not fromlist: + yield level, modname_s - if imported: - yield modname, imported +def scan(module_name, module_path, search_path): + module = Module(module_name, module_path) + stack = [module] + seen = set() -module = Module(name='ansible_module_apt', path='/Users/dmw/src/mitogen/.venv/lib/python2.7/site-packages/ansible/modules/packaging/os/apt.py') -path = tuple(sys.path) -path = ('/Users/dmw/src/ansible/lib',) + path + while stack: + module = stack.pop(0) + for level, fromname in scan_fromlist(module.code()): + if not fromname.startswith(PREFIX): + continue + imported = find(fromname[len(PREFIX):], search_path) + if imported is None or imported in seen: + continue -from pprint import pprint -for name, imported in scan(module, sys.path): - print '%s: %s' % (name, imported and (str(name) == imported.fullname())) + seen.add(imported) + parent = imported.parent + while parent: + module = Module(name=parent.fullname(), path=parent.path, + kind=parent.kind) + if module not in seen: + seen.add(module) + stack.append(module) + parent = parent.parent + + return sorted( + (PREFIX + module.fullname(), module.path, module.is_pkg) + for module in seen + ) From 81b62d9a1aa19fbdf406f346002e1b6db22f88e1 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Sat, 12 May 2018 14:14:35 +0100 Subject: [PATCH 06/59] issue #217: ansible: beginnings of ModuleDepService. --- ansible_mitogen/planner.py | 49 ++++++++++++++++++++++++++++++++++++- ansible_mitogen/process.py | 1 + ansible_mitogen/runner.py | 4 +++ ansible_mitogen/services.py | 42 +++++++++++++++++++++++++++++++ 4 files changed, 95 insertions(+), 1 deletion(-) diff --git a/ansible_mitogen/planner.py b/ansible_mitogen/planner.py index f19d77d4..aa96bd77 100644 --- a/ansible_mitogen/planner.py +++ b/ansible_mitogen/planner.py @@ -35,17 +35,19 @@ files/modules known missing. """ from __future__ import absolute_import -import json import logging import os from ansible.executor import module_common import ansible.errors +import ansible.module_utils try: from ansible.plugins.loader import module_loader + from ansible.plugins.loader import module_utils_loader except ImportError: # Ansible <2.4 from ansible.plugins import module_loader + from ansible.plugins import module_utils_loader import mitogen import mitogen.service @@ -281,6 +283,51 @@ class NewStylePlanner(ScriptPlanner): def detect(self, invocation): return 'from ansible.module_utils.' in invocation.module_source + def get_module_utils_path(self, invocation): + paths = [ + path + for path in module_utils_loader._get_paths(subdirs=False) + if os.path.isdir(path) + ] + paths.append(module_common._MODULE_UTILS_PATH) + return tuple(paths) + + def get_module_utils(self, invocation): + module_utils = mitogen.service.call( + context=invocation.connection.parent, + handle=ansible_mitogen.services.ModuleDepService.handle, + method='scan', + kwargs={ + 'module_name': 'ansible_module_%s' % (invocation.module_name,), + 'module_path': invocation.module_path, + 'search_path': self.get_module_utils_path(invocation), + } + ) + modutils_dir = os.path.dirname(ansible.module_utils.__file__) + has_custom = not all(path.startswith(modutils_dir) + for fullname, path, is_pkg in module_utils) + return module_utils, has_custom + + def plan(self, invocation): + invocation.connection._connect() + module_utils, has_custom = self.get_module_utils(invocation) + mitogen.service.call( + context=invocation.connection.parent, + handle=ansible_mitogen.services.FileService.handle, + method='register_many', + kwargs={ + 'paths': [ + path + for fullname, path, is_pkg in module_utils + ] + } + ) + return super(NewStylePlanner, self).plan( + invocation, + module_utils=module_utils, + should_fork=(self.get_should_fork(invocation) or has_custom), + ) + class ReplacerPlanner(NewStylePlanner): """ diff --git a/ansible_mitogen/process.py b/ansible_mitogen/process.py index f5dc7be5..82b6a59c 100644 --- a/ansible_mitogen/process.py +++ b/ansible_mitogen/process.py @@ -156,6 +156,7 @@ class MuxProcess(object): services=[ ansible_mitogen.services.ContextService(self.router), ansible_mitogen.services.FileService(self.router), + ansible_mitogen.services.ModuleDepService(self.router), ], size=int(os.environ.get('MITOGEN_POOL_SIZE', '16')), ) diff --git a/ansible_mitogen/runner.py b/ansible_mitogen/runner.py index bd2b0cc7..e37c9326 100644 --- a/ansible_mitogen/runner.py +++ b/ansible_mitogen/runner.py @@ -402,6 +402,10 @@ class NewStyleRunner(ScriptRunner): #: path => new-style module bytecode. _code_by_path = {} + def __init__(self, module_utils, **kwargs): + super(NewStyleRunner, self).__init__(**kwargs) + self.module_utils = module_utils + def setup(self): super(NewStyleRunner, self).setup() self._stdio = NewStyleStdio(self.args) diff --git a/ansible_mitogen/services.py b/ansible_mitogen/services.py index 38bca7da..55c0365b 100644 --- a/ansible_mitogen/services.py +++ b/ansible_mitogen/services.py @@ -49,7 +49,9 @@ import threading import zlib import mitogen +import mitogen.master import mitogen.service +import ansible_mitogen.module_finder import ansible_mitogen.target @@ -440,6 +442,17 @@ class FileService(mitogen.service.Service): except KeyError: return None + @mitogen.service.expose(policy=mitogen.service.AllowParents()) + @mitogen.service.arg_spec({ + 'paths': list + }) + def register_many(self, paths): + """ + Batch version of register(). + """ + for path in paths: + self.register(path) + @mitogen.service.expose(policy=mitogen.service.AllowParents()) @mitogen.service.arg_spec({ 'path': basestring @@ -589,3 +602,32 @@ class FileService(mitogen.service.Service): self._schedule_pending_unlocked(state) finally: state.lock.release() + + +class ModuleDepService(mitogen.service.Service): + """ + Scan a new-style module and produce a cached mapping of module_utils names + to their resolved filesystem paths. + """ + max_message_size = 1000 + handle = 502 + + def __init__(self, *args, **kwargs): + super(ModuleDepService, self).__init__(*args, **kwargs) + self._cache = {} + + @mitogen.service.expose(policy=mitogen.service.AllowParents()) + @mitogen.service.arg_spec({ + 'module_name': basestring, + 'module_path': basestring, + 'search_path': tuple, + }) + def scan(self, module_name, module_path, search_path): + if (module_name, search_path) not in self._cache: + resolved = ansible_mitogen.module_finder.scan( + module_name=module_name, + module_path=module_path, + search_path=search_path, + ) + self._cache[module_name, search_path] = resolved + return self._cache[module_name, search_path] From 30034877a5c770baa65574cde38a60b539e539bd Mon Sep 17 00:00:00 2001 From: David Wilson Date: Sun, 13 May 2018 01:47:11 +0100 Subject: [PATCH 07/59] issue #217: ansible: working, if extremely inefficient implementation --- ansible_mitogen/module_finder.py | 64 ++++++++++++++++++-------------- ansible_mitogen/planner.py | 24 ++++++------ ansible_mitogen/process.py | 8 +++- ansible_mitogen/runner.py | 45 ++++++++++++++++++++++ ansible_mitogen/services.py | 12 +++++- 5 files changed, 108 insertions(+), 45 deletions(-) diff --git a/ansible_mitogen/module_finder.py b/ansible_mitogen/module_finder.py index dd20237d..44c06a3c 100644 --- a/ansible_mitogen/module_finder.py +++ b/ansible_mitogen/module_finder.py @@ -1,5 +1,6 @@ from __future__ import absolute_import +import collections import imp import os import sys @@ -11,33 +12,27 @@ import ansible.module_utils PREFIX = 'ansible.module_utils.' -class Module(object): - def __init__(self, name, path, kind=imp.PY_SOURCE, parent=None): - self.name = name - self.path = path - self.kind = kind - self.is_pkg = kind == imp.PKG_DIRECTORY - self.parent = parent +Module = collections.namedtuple('Module', 'name path kind parent') - def __hash__(self): - return hash(self.path) - def __eq__(self, other): - return self.path == other.path +def get_fullname(module): + bits = [str(module.name)] + while module.parent: + bits.append(str(module.parent.name)) + module = module.parent + return '.'.join(reversed(bits)) - def fullname(self): - bits = [str(self.name)] - while self.parent: - bits.append(str(self.parent.name)) - self = self.parent - return '.'.join(reversed(bits)) - def code(self): - fp = open(self.path) - try: - return compile(fp.read(), str(self.name), 'exec') - finally: - fp.close() +def get_code(module): + fp = open(module.path) + try: + return compile(fp.read(), str(module.name), 'exec') + finally: + fp.close() + + +def is_pkg(module): + return module.kind == imp.PKG_DIRECTORY def find(name, path=(), parent=None): @@ -76,13 +71,18 @@ def scan_fromlist(code): def scan(module_name, module_path, search_path): - module = Module(module_name, module_path) + module = Module( + name=module_name, + path=module_path, + kind=imp.PY_SOURCE, + parent=None, + ) stack = [module] seen = set() while stack: module = stack.pop(0) - for level, fromname in scan_fromlist(module.code()): + for level, fromname in scan_fromlist(get_code(module)): if not fromname.startswith(PREFIX): continue @@ -90,17 +90,25 @@ def scan(module_name, module_path, search_path): if imported is None or imported in seen: continue + if imported in seen: + continue + seen.add(imported) + stack.append(imported) parent = imported.parent while parent: - module = Module(name=parent.fullname(), path=parent.path, - kind=parent.kind) + module = Module( + name=get_fullname(parent), + path=parent.path, + kind=parent.kind, + parent=None, + ) if module not in seen: seen.add(module) stack.append(module) parent = parent.parent return sorted( - (PREFIX + module.fullname(), module.path, module.is_pkg) + (PREFIX + get_fullname(module), module.path, is_pkg(module)) for module in seen ) diff --git a/ansible_mitogen/planner.py b/ansible_mitogen/planner.py index aa96bd77..0e4499e9 100644 --- a/ansible_mitogen/planner.py +++ b/ansible_mitogen/planner.py @@ -180,7 +180,7 @@ class BinaryPlanner(Planner): def detect(self, invocation): return module_common._is_binary(invocation.module_source) - def plan(self, invocation, **kwargs): + def _grant_file_service_access(self, invocation): invocation.connection._connect() mitogen.service.call( context=invocation.connection.parent, @@ -190,6 +190,9 @@ class BinaryPlanner(Planner): 'path': invocation.module_path } ) + + def plan(self, invocation, **kwargs): + self._grant_file_service_access(invocation) return super(BinaryPlanner, self).plan( invocation=invocation, runner_name=self.runner_name, @@ -270,6 +273,12 @@ class NewStylePlanner(ScriptPlanner): def _get_interpreter(self, invocation): return None, None + def _grant_file_service_access(self, invocation): + """ + Stub out BinaryPlanner's method since ModuleDepService makes internal + calls to grant file access, avoiding 2 IPCs per task invocation. + """ + def get_should_fork(self, invocation): """ In addition to asynchronous tasks, new-style modules should be forked @@ -293,6 +302,7 @@ class NewStylePlanner(ScriptPlanner): return tuple(paths) def get_module_utils(self, invocation): + invocation.connection._connect() module_utils = mitogen.service.call( context=invocation.connection.parent, handle=ansible_mitogen.services.ModuleDepService.handle, @@ -309,19 +319,7 @@ class NewStylePlanner(ScriptPlanner): return module_utils, has_custom def plan(self, invocation): - invocation.connection._connect() module_utils, has_custom = self.get_module_utils(invocation) - mitogen.service.call( - context=invocation.connection.parent, - handle=ansible_mitogen.services.FileService.handle, - method='register_many', - kwargs={ - 'paths': [ - path - for fullname, path, is_pkg in module_utils - ] - } - ) return super(NewStylePlanner, self).plan( invocation, module_utils=module_utils, diff --git a/ansible_mitogen/process.py b/ansible_mitogen/process.py index 82b6a59c..4946aa29 100644 --- a/ansible_mitogen/process.py +++ b/ansible_mitogen/process.py @@ -151,12 +151,16 @@ class MuxProcess(object): Construct a ContextService and a thread to service requests for it arriving from worker processes. """ + file_service = ansible_mitogen.services.FileService(router=self.router) self.pool = mitogen.service.Pool( router=self.router, services=[ + file_service, ansible_mitogen.services.ContextService(self.router), - ansible_mitogen.services.FileService(self.router), - ansible_mitogen.services.ModuleDepService(self.router), + ansible_mitogen.services.ModuleDepService( + router=self.router, + file_service=file_service, + ), ], size=int(os.environ.get('MITOGEN_POOL_SIZE', '16')), ) diff --git a/ansible_mitogen/runner.py b/ansible_mitogen/runner.py index e37c9326..807623f7 100644 --- a/ansible_mitogen/runner.py +++ b/ansible_mitogen/runner.py @@ -38,6 +38,7 @@ how to build arguments for it, preseed related data, etc. from __future__ import absolute_import import cStringIO import ctypes +import imp import json import logging import os @@ -182,6 +183,46 @@ class Runner(object): self.revert() +class ModuleUtilsImporter(object): + """ + :param list module_utils: + List of `(fullname, path, is_pkg)` tuples. + """ + def __init__(self, context, module_utils): + self._context = context + self._by_fullname = { + fullname: (path, is_pkg) + for fullname, path, is_pkg in module_utils + } + self._loaded = set() + sys.meta_path.insert(0, self) + + def revert(self): + sys.meta_path.remove(self) + for fullname in self._loaded: + sys.modules.pop(fullname, None) + + def find_module(self, fullname, path=None): + if fullname in self._by_fullname: + return self + + def load_module(self, fullname): + path, is_pkg = self._by_fullname[fullname] + source = ansible_mitogen.target.get_file(self._context, path) + code = compile(source, path, 'exec') + mod = sys.modules.setdefault(fullname, imp.new_module(fullname)) + mod.__file__ = "master:%s" % (path,) + mod.__loader__ = self + if is_pkg: + mod.__path__ = [] + mod.__package__ = fullname + else: + mod.__package__ = fullname.rpartition('.')[0] + exec(code, mod.__dict__) + self._loaded.add(fullname) + return mod + + class TemporaryEnvironment(object): def __init__(self, env=None): self.original = os.environ.copy() @@ -413,6 +454,10 @@ class NewStyleRunner(ScriptRunner): # module, but this has never been a bug report. Instead act like an # interpreter that had its script piped on stdin. self._argv = TemporaryArgv(['']) + self._importer = ModuleUtilsImporter( + context=self.service_context, + module_utils=self.module_utils, + ) if libc__res_init: libc__res_init() diff --git a/ansible_mitogen/services.py b/ansible_mitogen/services.py index 55c0365b..e7d6b2d7 100644 --- a/ansible_mitogen/services.py +++ b/ansible_mitogen/services.py @@ -612,8 +612,9 @@ class ModuleDepService(mitogen.service.Service): max_message_size = 1000 handle = 502 - def __init__(self, *args, **kwargs): - super(ModuleDepService, self).__init__(*args, **kwargs) + def __init__(self, file_service, **kwargs): + super(ModuleDepService, self).__init__(**kwargs) + self._file_service = file_service self._cache = {} @mitogen.service.expose(policy=mitogen.service.AllowParents()) @@ -630,4 +631,11 @@ class ModuleDepService(mitogen.service.Service): search_path=search_path, ) self._cache[module_name, search_path] = resolved + + # Grant FileService access to paths in here to avoid another 2 IPCs + # from WorkerProcess. + self._file_service.register(path=module_path) + for fullname, path, is_pkg in resolved: + self._file_service.register(path=path) + return self._cache[module_name, search_path] From bb61745a1a13e59ef0d95b665284ad169a9ae2cc Mon Sep 17 00:00:00 2001 From: David Wilson Date: Sun, 13 May 2018 03:18:22 +0100 Subject: [PATCH 08/59] issue #217: pass through non-custom module utils to regular importer. This may come back to bite later, but in the meantime it avoids shipping up to 12KiB of junk metadata for every single task invocation. For detachment (aka. async), we must ensure the target has two types of preloads completed (modules and module_utils files) before detaching. --- ansible_mitogen/module_finder.py | 66 +++++++++++++++++++++++--------- ansible_mitogen/planner.py | 21 ++++------ ansible_mitogen/services.py | 15 ++++++-- ansible_mitogen/target.py | 2 +- 4 files changed, 68 insertions(+), 36 deletions(-) diff --git a/ansible_mitogen/module_finder.py b/ansible_mitogen/module_finder.py index 44c06a3c..28522dd9 100644 --- a/ansible_mitogen/module_finder.py +++ b/ansible_mitogen/module_finder.py @@ -1,12 +1,37 @@ +# Copyright 2017, David Wilson +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are met: +# +# 1. Redistributions of source code must retain the above copyright notice, +# this list of conditions and the following disclaimer. +# +# 2. Redistributions in binary form must reproduce the above copyright notice, +# this list of conditions and the following disclaimer in the documentation +# and/or other materials provided with the distribution. +# +# 3. Neither the name of the copyright holder nor the names of its contributors +# may be used to endorse or promote products derived from this software without +# specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" +# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE +# ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE +# LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR +# CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF +# SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS +# INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN +# CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) +# ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE +# POSSIBILITY OF SUCH DAMAGE. from __future__ import absolute_import import collections import imp import os -import sys import mitogen.master -import ansible.module_utils PREFIX = 'ansible.module_utils.' @@ -16,6 +41,9 @@ Module = collections.namedtuple('Module', 'name path kind parent') def get_fullname(module): + """ + Reconstruct a Module's canonical path by recursing through its parents. + """ bits = [str(module.name)] while module.parent: bits.append(str(module.parent.name)) @@ -24,6 +52,9 @@ def get_fullname(module): def get_code(module): + """ + Compile and return a Module's code object. + """ fp = open(module.path) try: return compile(fp.read(), str(module.name), 'exec') @@ -32,12 +63,23 @@ def get_code(module): def is_pkg(module): + """ + Return :data:`True` if a Module represents a package. + """ return module.kind == imp.PKG_DIRECTORY def find(name, path=(), parent=None): """ - (Name, search path) -> Module instance or None. + Return a Module instance describing the first matching module found on the + given search path. + + :param str name: + Module name. + :param str path: + Search path. + :param Module parent: + If given, make the found module a child of this module. """ head, _, tail = name.partition('.') try: @@ -71,12 +113,7 @@ def scan_fromlist(code): def scan(module_name, module_path, search_path): - module = Module( - name=module_name, - path=module_path, - kind=imp.PY_SOURCE, - parent=None, - ) + module = Module(module_name, module_path, imp.PY_SOURCE, None) stack = [module] seen = set() @@ -90,19 +127,12 @@ def scan(module_name, module_path, search_path): if imported is None or imported in seen: continue - if imported in seen: - continue - seen.add(imported) stack.append(imported) parent = imported.parent while parent: - module = Module( - name=get_fullname(parent), - path=parent.path, - kind=parent.kind, - parent=None, - ) + fullname = get_fullname(parent) + module = Module(fullname, parent.path, parent.kind, None) if module not in seen: seen.add(module) stack.append(module) diff --git a/ansible_mitogen/planner.py b/ansible_mitogen/planner.py index 0e4499e9..e1f37fcd 100644 --- a/ansible_mitogen/planner.py +++ b/ansible_mitogen/planner.py @@ -292,38 +292,33 @@ class NewStylePlanner(ScriptPlanner): def detect(self, invocation): return 'from ansible.module_utils.' in invocation.module_source - def get_module_utils_path(self, invocation): - paths = [ + def get_search_path(self, invocation): + return tuple( path for path in module_utils_loader._get_paths(subdirs=False) if os.path.isdir(path) - ] - paths.append(module_common._MODULE_UTILS_PATH) - return tuple(paths) + ) def get_module_utils(self, invocation): invocation.connection._connect() - module_utils = mitogen.service.call( + return mitogen.service.call( context=invocation.connection.parent, handle=ansible_mitogen.services.ModuleDepService.handle, method='scan', kwargs={ 'module_name': 'ansible_module_%s' % (invocation.module_name,), 'module_path': invocation.module_path, - 'search_path': self.get_module_utils_path(invocation), + 'search_path': self.get_search_path(invocation), + 'builtin_path': module_common._MODULE_UTILS_PATH, } ) - modutils_dir = os.path.dirname(ansible.module_utils.__file__) - has_custom = not all(path.startswith(modutils_dir) - for fullname, path, is_pkg in module_utils) - return module_utils, has_custom def plan(self, invocation): - module_utils, has_custom = self.get_module_utils(invocation) + module_utils = self.get_module_utils(invocation) return super(NewStylePlanner, self).plan( invocation, module_utils=module_utils, - should_fork=(self.get_should_fork(invocation) or has_custom), + should_fork=(self.get_should_fork(invocation) or bool(module_utils)), ) diff --git a/ansible_mitogen/services.py b/ansible_mitogen/services.py index e7d6b2d7..0f5be35d 100644 --- a/ansible_mitogen/services.py +++ b/ansible_mitogen/services.py @@ -622,20 +622,27 @@ class ModuleDepService(mitogen.service.Service): 'module_name': basestring, 'module_path': basestring, 'search_path': tuple, + 'builtin_path': basestring, }) - def scan(self, module_name, module_path, search_path): + def scan(self, module_name, module_path, search_path, builtin_path): if (module_name, search_path) not in self._cache: resolved = ansible_mitogen.module_finder.scan( module_name=module_name, module_path=module_path, - search_path=search_path, + search_path=tuple(search_path) + (builtin_path,), ) - self._cache[module_name, search_path] = resolved + builtin_path = os.path.abspath(builtin_path) + filtered = [ + (fullname, path, is_pkg) + for fullname, path, is_pkg in resolved + if not os.path.abspath(path).startswith(builtin_path) + ] + self._cache[module_name, search_path] = filtered # Grant FileService access to paths in here to avoid another 2 IPCs # from WorkerProcess. self._file_service.register(path=module_path) - for fullname, path, is_pkg in resolved: + for fullname, path, is_pkg in filtered: self._file_service.register(path=path) return self._cache[module_name, search_path] diff --git a/ansible_mitogen/target.py b/ansible_mitogen/target.py index 5dc1dfa7..05deacf0 100644 --- a/ansible_mitogen/target.py +++ b/ansible_mitogen/target.py @@ -518,7 +518,7 @@ def write_path(path, s, owner=None, group=None, mode=None, prefix='.ansible_mitogen_transfer-', dir=os.path.dirname(path)) fp = os.fdopen(fd, 'wb', mitogen.core.CHUNK_SIZE) - LOG.debug('write_path(path=%r) tempory file: %s', path, tmp_path) + LOG.debug('write_path(path=%r) temporary file: %s', path, tmp_path) try: try: From 8a089e975d9151f43ff297bff19cb013fcdcbb2a Mon Sep 17 00:00:00 2001 From: David Wilson Date: Sun, 13 May 2018 15:58:37 +0100 Subject: [PATCH 09/59] docs: Document Router.unidirectional. --- docs/api.rst | 100 ++++++++++++++++++++++++++++++++------------------- 1 file changed, 64 insertions(+), 36 deletions(-) diff --git a/docs/api.rst b/docs/api.rst index a1711f11..365edcb6 100644 --- a/docs/api.rst +++ b/docs/api.rst @@ -80,9 +80,9 @@ contexts. :py:class:`mitogen.core.Receiver` or :py:class:`mitogen.master.Select` instances and returns the first value posted to any receiver or select. - If `oneshot` is ``True``, then remove each receiver as it yields a result; - since :py:meth:`__iter__` terminates once the final receiver is removed, - this makes it convenient to respond to calls made in parallel: + If `oneshot` is :data:`True`, then remove each receiver as it yields a + result; since :py:meth:`__iter__` terminates once the final receiver is + removed, this makes it convenient to respond to calls made in parallel: .. code-block:: python @@ -173,7 +173,7 @@ contexts. .. py:method:: __bool__ () - Return ``True`` if any receivers are registered with this select. + Return :data:`True` if any receivers are registered with this select. .. py:method:: close () @@ -188,11 +188,12 @@ contexts. .. py:method:: empty () - Return ``True`` if calling :py:meth:`get` would block. + Return :data:`True` if calling :py:meth:`get` would block. - As with :py:class:`Queue.Queue`, ``True`` may be returned even though a - subsequent call to :py:meth:`get` will succeed, since a message may be - posted at any moment between :py:meth:`empty` and :py:meth:`get`. + As with :py:class:`Queue.Queue`, :data:`True` may be returned even + though a subsequent call to :py:meth:`get` will succeed, since a + message may be posted at any moment between :py:meth:`empty` and + :py:meth:`get`. :py:meth:`empty` may return ``False`` even when :py:meth:`get` would block if another thread has drained a receiver added to this select. @@ -202,7 +203,7 @@ contexts. .. py:method:: __iter__ (self) Yield the result of :py:meth:`get` until no receivers remain in the - select, either because `oneshot` is ``True``, or each receiver was + select, either because `oneshot` is :data:`True`, or each receiver was explicitly removed via :py:meth:`remove`. .. py:method:: add (recv) @@ -402,6 +403,24 @@ Router Class **Note:** This is the somewhat limited core version of the Router class used by child contexts. The master subclass is documented below this one. + .. attribute:: unidirectional + + When :data:`True`, permit children to only communicate with the current + context or a parent of the current context. Routing between siblings or + children of parents is prohibited, ensuring no communication is + possible between intentionally partitioned networks, such as when a + program simultaneously manipulates hosts spread across a corporate and + a production network, or production networks that are otherwise + air-gapped. + + Sending a prohibited message causes an error to be logged and a dead + message to be sent in reply to the errant message, if that message has + ``reply_to`` set. + + The value of :data:`unidirectional` becomes the default for the + :meth:`local() ` `unidirectional` + parameter. + .. method:: stream_by_id (dst_id) Return the :py:class:`mitogen.core.Stream` that should be used to @@ -523,13 +542,13 @@ Router Class :py:class:`Broker` instance to use. If not specified, a private :py:class:`Broker` is created. - .. data:: profiling + .. attribute:: profiling - When enabled, causes the broker thread and any subsequent broker and - main threads existing in any child to write + When :data:`True`, cause the broker thread and any subsequent broker + and main threads existing in any child to write ``/tmp/mitogen.stats...log`` containing a - :py:mod:`cProfile` dump on graceful exit. Must be set prior to any - :py:class:`Broker` being constructed, e.g. via: + :py:mod:`cProfile` dump on graceful exit. Must be set prior to + construction of any :py:class:`Broker`, e.g. via: .. code:: @@ -674,19 +693,26 @@ Router Class ``python2.7``. In future this may default to ``sys.executable``. :param bool debug: - If ``True``, arrange for debug logging (:py:meth:`enable_debug`) to - be enabled in the new context. Automatically ``True`` when + If :data:`True`, arrange for debug logging (:py:meth:`enable_debug`) to + be enabled in the new context. Automatically :data:`True` when :py:meth:`enable_debug` has been called, but may be used selectively otherwise. + :param bool unidirectional: + If :data:`True`, arrange for the child's router to be constructed + with :attr:`unidirectional routing + ` enabled. Automatically + :data:`True` when it was enabled for this router, but may still be + explicitly set to :data:`False`. + :param float connect_timeout: Fractional seconds to wait for the subprocess to indicate it is healthy. Defaults to 30 seconds. :param bool profiling: - If ``True``, arrange for profiling (:py:data:`profiling`) to be - enabled in the new context. Automatically ``True`` when - :py:data:`profiling` is ``True``, but may be used selectively + If :data:`True`, arrange for profiling (:py:data:`profiling`) to be + enabled in the new context. Automatically :data:`True` when + :py:data:`profiling` is :data:`True`, but may be used selectively otherwise. :param mitogen.core.Context via: @@ -1067,8 +1093,8 @@ Receiver Class handle is chosen. :param bool persist: - If ``True``, do not unregister the receiver's handler after the first - message. + If :data:`True`, do not unregister the receiver's handler after the + first message. :param mitogen.core.Context respondent: Reference to the context this receiver is receiving from. If not @@ -1102,11 +1128,12 @@ Receiver Class .. py:method:: empty () - Return ``True`` if calling :py:meth:`get` would block. + Return :data:`True` if calling :py:meth:`get` would block. - As with :py:class:`Queue.Queue`, ``True`` may be returned even though a - subsequent call to :py:meth:`get` will succeed, since a message may be - posted at any moment between :py:meth:`empty` and :py:meth:`get`. + As with :py:class:`Queue.Queue`, :data:`True` may be returned even + though a subsequent call to :py:meth:`get` will succeed, since a + message may be posted at any moment between :py:meth:`empty` and + :py:meth:`get`. :py:meth:`empty` is only useful to avoid a race while installing :py:attr:`notify`: @@ -1256,10 +1283,10 @@ Broker Class .. method:: keep_alive - Return ``True`` if any reader's :py:attr:`Side.keep_alive` attribute is - ``True``, or any :py:class:`Context` is still registered that is not - the master. Used to delay shutdown while some important work is in - progress (e.g. log draining). + Return :data:`True` if any reader's :py:attr:`Side.keep_alive` + attribute is :data:`True`, or any :py:class:`Context` is still + registered that is not the master. Used to delay shutdown while some + important work is in progress (e.g. log draining). **Internal Methods** @@ -1284,9 +1311,10 @@ Broker Class non-payment results in termination for one customer. :param bool install_watcher: - If ``True``, an additional thread is started to monitor the lifetime of - the main thread, triggering :py:meth:`shutdown` automatically in case - the user forgets to call it, or their code crashed. + If :data:`True`, an additional thread is started to monitor the + lifetime of the main thread, triggering :py:meth:`shutdown` + automatically in case the user forgets to call it, or their code + crashed. You should not rely on this functionality in your program, it is only intended as a fail-safe and to simplify the API for new users. In @@ -1348,12 +1376,12 @@ A random assortment of utility functions useful on masters and children. are written to :py:data:`sys.stderr`. :param bool io: - If ``True``, include extremely verbose IO logs in the output. Useful - for debugging hangs, less useful for debugging application code. + If :data:`True`, include extremely verbose IO logs in the output. + Useful for debugging hangs, less useful for debugging application code. :parm bool usec: - If ``True``, include microsecond timestamps. This greatly helps when - debugging races and similar determinism issues. + If :data:`True`, include microsecond timestamps. This greatly helps + when debugging races and similar determinism issues. :param str level: Name of the :py:mod:`logging` package constant that is the minimum From d1a22cb5d44983787a8a6e884d4577004810a55e Mon Sep 17 00:00:00 2001 From: David Wilson Date: Sun, 13 May 2018 16:58:47 +0100 Subject: [PATCH 10/59] issue #186: parent: implement FORWARD_MODULE. To support detach, we must be able to preload the target with every module it will need prior to detachment. This implements the intermediary part of the process (i.e. the Ansible fork parent) -- receiving LOAD_MODULE/FORWARD_MODULE pairs and ensuring they reach the child. --- docs/howitworks.rst | 17 ++++++++++ mitogen/core.py | 3 +- mitogen/parent.py | 76 +++++++++++++++++++++++++++++++++------------ 3 files changed, 75 insertions(+), 21 deletions(-) diff --git a/docs/howitworks.rst b/docs/howitworks.rst index 5c8bb018..dfe2b2d7 100644 --- a/docs/howitworks.rst +++ b/docs/howitworks.rst @@ -445,6 +445,23 @@ also listen on the following handles: In this way, the master need never re-send a module it has already sent to a direct descendant. +.. currentmodule:: mitogen.core +.. data:: FORWARD_MODULE + + Receives `(context, fullname)` tuples from its parent and arranges for a + :data:`LOAD_MODULE` to be sent towards `context` for the module `fullname` + and any related modules. The module must already have been delivered to the + current context by its parent in a prior :data:`LOAD_MODULE` message. + + If the receiver is the immediate parent of `context`, then only + :data:`LOAD_MODULE` is sent to the child. Otherwise :data:`LOAD_MODULE` is + sent to the next closest parent if the module has not previously been sent + on that stream, followed by a copy of the :data:`FORWARD_MODULE` message. + + This message is used to recursively preload indirect children with modules, + ensuring they are cached and deduplicated at each hop in the chain leading + to the target context. + .. currentmodule:: mitogen.core .. data:: DETACHING diff --git a/mitogen/core.py b/mitogen/core.py index 26151cbf..e947ecfe 100644 --- a/mitogen/core.py +++ b/mitogen/core.py @@ -75,7 +75,8 @@ DEL_ROUTE = 104 ALLOCATE_ID = 105 SHUTDOWN = 106 LOAD_MODULE = 107 -DETACHING = 108 +FORWARD_MODULE = 108 +DETACHING = 109 IS_DEAD = 999 try: diff --git a/mitogen/parent.py b/mitogen/parent.py index 5cdc89c9..e6d76c6b 100644 --- a/mitogen/parent.py +++ b/mitogen/parent.py @@ -1106,6 +1106,12 @@ class ModuleForwarder(object): self.router = router self.parent_context = parent_context self.importer = importer + router.add_handler( + fn=self._on_forward_module, + handle=mitogen.core.FORWARD_MODULE, + persist=True, + policy=mitogen.core.has_parent_authority, + ) router.add_handler( fn=self._on_get_module, handle=mitogen.core.GET_MODULE, @@ -1116,34 +1122,64 @@ class ModuleForwarder(object): def __repr__(self): return 'ModuleForwarder(%r)' % (self.router,) + def _on_forward_module(self, msg): + if msg.is_dead: + return + + context_id_s, fullname = msg.data.partition('\x00') + context_id = int(context_id_s) + stream = self.router.stream_by_id(context_id) + if stream.remote_id == mitogen.parent_id: + LOG.error('%r: dropping FORWARD_MODULE(%d, %r): no route to child', + self, context_id, fullname) + return + + LOG.debug('%r._on_forward_module() sending %r to %r via %r', + self, fullname, context_id, stream.remote_id) + self._send_module_and_related(stream, fullname) + if stream.remote_id != context_id: + stream._send( + mitogen.core.Message( + dst_id=stream.remote_id, + handle=mitogen.core.FORWARD_MODULE, + data=msg.data, + ) + ) + def _on_get_module(self, msg): LOG.debug('%r._on_get_module(%r)', self, msg) if msg.is_dead: return fullname = msg.data - callback = lambda: self._on_cache_callback(msg, fullname) - self.importer._request_module(fullname, callback) - - def _send_one_module(self, msg, tup): - self.router._async_route( - mitogen.core.Message.pickled( - tup, - dst_id=msg.src_id, - handle=mitogen.core.LOAD_MODULE, - ) + self.importer._request_module(fullname, + lambda: self._on_cache_callback(msg, fullname) ) def _on_cache_callback(self, msg, fullname): LOG.debug('%r._on_get_module(): sending %r', self, fullname) + stream = self.router.stream_by_id(msg.src_id) + self._send_module_and_related(stream, fullname) + + def _send_module_and_related(self, stream, fullname): tup = self.importer._cache[fullname] - if tup is not None: - for related in tup[4]: - rtup = self.importer._cache.get(related) - if not rtup: - LOG.debug('%r._on_get_module(): skipping absent %r', - self, related) - continue - self._send_one_module(msg, rtup) - - self._send_one_module(msg, tup) + for related in tup[4]: + rtup = self.importer._cache.get(related) + if rtup: + self._send_one_module(stream, rtup) + else: + LOG.debug('%r._send_module_and_related(%r): absent: %r', + self, fullname, related) + + self._send_one_module(stream, tup) + + def _send_one_module(self, stream, tup): + if tup[0] not in stream.sent_modules: + stream.sent_modules.add(tup[0]) + self.router._async_route( + mitogen.core.Message.pickled( + tup, + dst_id=stream.remote_id, + handle=mitogen.core.LOAD_MODULE, + ) + ) From b7ab473343f6b4f3df05bd4dbe39425a5ea814b2 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Sun, 13 May 2018 17:05:09 +0100 Subject: [PATCH 11/59] issue #186: split handle list up so it makes sense --- docs/howitworks.rst | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/docs/howitworks.rst b/docs/howitworks.rst index dfe2b2d7..dde63c14 100644 --- a/docs/howitworks.rst +++ b/docs/howitworks.rst @@ -434,6 +434,22 @@ also listen on the following handles: route from its local table, then propagates the message upward towards its own parent. +.. currentmodule:: mitogen.core +.. data:: DETACHING + + Sent to inform a parent that user code has invoked + :meth:`ExternalContext.detach` to decouple the lifecycle of a directly + connected context and its subtree from the running program. + + A child usually shuts down immediately if it loses its parent connection, + and parents usually terminate any related Python/SSH subprocess on + disconnection. Receiving :data:`DETACHING` informs the parent the + connection will soon drop, but the process intends to continue life + independently, and to avoid terminating the related subprocess if that + subprocess is the child itself. + +Non-master parents also listen on the following handles: + .. currentmodule:: mitogen.core .. data:: GET_MODULE @@ -462,20 +478,6 @@ also listen on the following handles: ensuring they are cached and deduplicated at each hop in the chain leading to the target context. -.. currentmodule:: mitogen.core -.. data:: DETACHING - - Sent to inform a parent that user code has invoked - :meth:`ExternalContext.detach` to decouple the lifecycle of a directly - connected context and its subtree from the running program. - - A child usually shuts down immediately if it loses its parent connection, - and parents usually terminate any related Python/SSH subprocess on - disconnection. Receiving :data:`DETACHING` informs the parent the - connection will soon drop, but the process intends to continue life - independently, and to avoid terminating the related subprocess if that - subprocess is the child itself. - Additional handles are created to receive the result of every function call triggered by :py:meth:`call_async() `. From bc7be1879d3697ec450c5619df523655ac025bbb Mon Sep 17 00:00:00 2001 From: David Wilson Date: Mon, 14 May 2018 18:28:05 +0100 Subject: [PATCH 12/59] issue #249: initial poller implementation (BSD only) --- docs/internals.rst | 13 +++++ mitogen/core.py | 111 ++++++++++++++++++++++++---------------- mitogen/master.py | 1 + mitogen/parent.py | 123 ++++++++++++++++++++++++++++++++++++++------- 4 files changed, 188 insertions(+), 60 deletions(-) diff --git a/docs/internals.rst b/docs/internals.rst index f3771343..3aa1fac4 100644 --- a/docs/internals.rst +++ b/docs/internals.rst @@ -261,6 +261,19 @@ Other Stream Subclasses :members: +Poller Class +------------ + +.. currentmodule:: mitogen.core +.. autoclass:: Poller + +.. currentmodule:: mitogen.parent +.. autoclass:: KqueuePoller + +.. currentmodule:: mitogen.parent +.. autoclass:: EpollPoller + + Importer Class -------------- diff --git a/mitogen/core.py b/mitogen/core.py index e947ecfe..cfea37d6 100644 --- a/mitogen/core.py +++ b/mitogen/core.py @@ -1437,24 +1437,15 @@ class Router(object): self.broker.defer(self._async_route, msg) -class Broker(object): - _waker = None - _thread = None - shutdown_timeout = 3.0 - +class Poller(object): def __init__(self): - self._alive = True - self._waker = Waker(self) - self.defer = self._waker.defer - self._readers = [self._waker.receive_side] - self._writers = [] - self._thread = threading.Thread( - target=_profile_hook, - args=('broker', self._broker_main), - name='mitogen-broker' - ) - self._thread.start() - self._waker.broker_ident = self._thread.ident + self.readers = [] + self.writers = [] + + _repr = 'Poller()' + + def __repr__(self): + return self._repr def _list_discard(self, lst, value): try: @@ -1469,63 +1460,97 @@ class Broker(object): def start_receive(self, stream): _vv and IOLOG.debug('%r.start_receive(%r)', self, stream) assert stream.receive_side and stream.receive_side.fd is not None - self.defer(self._list_add, self._readers, stream.receive_side) + self._list_add(self.readers, stream.receive_side) def stop_receive(self, stream): - IOLOG.debug('%r.stop_receive(%r)', self, stream) - self.defer(self._list_discard, self._readers, stream.receive_side) + _vv and IOLOG.debug('%r.stop_receive(%r)', self, stream) + self._list_discard(self.readers, stream.receive_side) - def _start_transmit(self, stream): - IOLOG.debug('%r._start_transmit(%r)', self, stream) + def start_transmit(self, stream): + _vv and IOLOG.debug('%r._start_transmit(%r)', self, stream) assert stream.transmit_side and stream.transmit_side.fd is not None - self._list_add(self._writers, stream.transmit_side) + self._list_add(self.writers, stream.transmit_side) - def _stop_transmit(self, stream): - IOLOG.debug('%r._stop_transmit(%r)', self, stream) - self._list_discard(self._writers, stream.transmit_side) + def stop_transmit(self, stream): + _vv and IOLOG.debug('%r._stop_transmit(%r)', self, stream) + self._list_discard(self.writers, stream.transmit_side) - def _call(self, stream, func): + def _call(self, broker, stream, func): try: - func(self) + func(broker) except Exception: LOG.exception('%r crashed', stream) stream.on_disconnect(self) - def _loop_once(self, timeout=None): - _vv and IOLOG.debug('%r._loop_once(%r)', self, timeout) - - #IOLOG.debug('readers = %r', self._readers) - #IOLOG.debug('writers = %r', self._writers) + def poll(self, broker, timeout=None): + _vv and IOLOG.debug('%r.poll(%r)', self, timeout) + #IOLOG.debug('readers = %r', self.readers) + #IOLOG.debug('writers = %r', self.writers) (rsides, wsides, _), _ = io_op(select.select, - self._readers, - self._writers, + self.readers, + self.writers, (), timeout ) for side in rsides: _vv and IOLOG.debug('%r: POLLIN for %r', self, side) - self._call(side.stream, side.stream.on_receive) + self._call(broker, side.stream, side.stream.on_receive) for side in wsides: _vv and IOLOG.debug('%r: POLLOUT for %r', self, side) - self._call(side.stream, side.stream.on_transmit) + self._call(broker, side.stream, side.stream.on_transmit) + + +class Broker(object): + poller_class = Poller + _waker = None + _thread = None + shutdown_timeout = 3.0 + + def __init__(self, poller_class=None): + self._alive = True + self._waker = Waker(self) + self.defer = self._waker.defer + self.poller = self.poller_class() + self.poller.start_receive(self._waker) + self.readers = [self._waker.receive_side] + self.writers = [] + self._thread = threading.Thread( + target=_profile_hook, + args=('broker', self._broker_main), + name='mitogen-broker' + ) + self._thread.start() + self._waker.broker_ident = self._thread.ident + + def start_receive(self, stream): + self.defer(self.poller.start_receive, stream) + + def stop_receive(self, stream): + self.defer(self.poller.stop_receive, stream) + + def _start_transmit(self, stream): + self.poller.start_transmit(stream) + + def _stop_transmit(self, stream): + self.poller.stop_transmit(stream) def keep_alive(self): - return sum((side.keep_alive for side in self._readers), 0) + return sum((side.keep_alive for side in self.poller.readers), 0) def _broker_main(self): try: while self._alive: - self._loop_once() + self.poller.poll(self) fire(self, 'shutdown') - for side in set(self._readers).union(self._writers): - self._call(side.stream, side.stream.on_shutdown) + for side in set(self.poller.readers).union(self.poller.writers): + self.poller._call(self, side.stream, side.stream.on_shutdown) deadline = time.time() + self.shutdown_timeout while self.keep_alive() and time.time() < deadline: - self._loop_once(max(0, deadline - time.time())) + self.poller.poll(self, max(0, deadline - time.time())) if self.keep_alive(): LOG.error('%r: some streams did not close gracefully. ' @@ -1533,7 +1558,7 @@ class Broker(object): 'more child processes still connected to ' 'our stdout/stderr pipes.', self) - for side in set(self._readers).union(self._writers): + for side in set(self.readers).union(self.writers): LOG.error('_broker_main() force disconnecting %r', side) side.stream.on_disconnect(self) except Exception: diff --git a/mitogen/master.py b/mitogen/master.py index 22117a50..0805206b 100644 --- a/mitogen/master.py +++ b/mitogen/master.py @@ -661,6 +661,7 @@ class ModuleResponder(object): class Broker(mitogen.core.Broker): shutdown_timeout = 5.0 _watcher = None + poller_class = mitogen.parent.PREFERRED_POLLER def __init__(self, install_watcher=True): if install_watcher: diff --git a/mitogen/parent.py b/mitogen/parent.py index e6d76c6b..835c7e79 100644 --- a/mitogen/parent.py +++ b/mitogen/parent.py @@ -70,23 +70,6 @@ except: SC_OPEN_MAX = 1024 -class Argv(object): - def __init__(self, argv): - self.argv = argv - - def escape(self, x): - s = '"' - for c in x: - if c in '\\$"`': - s += '\\' - s += c - s += '"' - return s - - def __str__(self): - return ' '.join(map(self.escape, self.argv)) - - def get_log_level(): return (LOG.level or logging.getLogger().level or logging.INFO) @@ -497,6 +480,112 @@ def _proxy_connect(name, method_name, kwargs, econtext): } +class Argv(object): + def __init__(self, argv): + self.argv = argv + + def escape(self, x): + s = '"' + for c in x: + if c in '\\$"`': + s += '\\' + s += c + s += '"' + return s + + def __str__(self): + return ' '.join(map(self.escape, self.argv)) + + + +class Poller(mitogen.core.Poller): + @classmethod + def from_existing(cls, poller): + self = cls() + for reader in poller.readers: + self.start_receive(reader.stream) + for writer in poller.writers: + self.start_transmit(writer.stream) + + +class KqueuePoller(Poller): + _repr = 'KqueuePoller()' + + def __init__(self): + self._kqueue = select.kqueue() + self._reader_by_fd = {} + self._writer_by_fd = {} + self._changelist = [] + + @property + def readers(self): + return list(self._reader_by_fd.values()) + + @property + def writers(self): + return list(self._writer_by_fd.values()) + + def _control(self, side, filters, flags): + mitogen.core._vv and IOLOG.debug( + '%r._control(%r, %r, %r)', self, side, filters, flags) + self._changelist.append(select.kevent(side.fd, filters, flags)) + + def start_receive(self, stream): + mitogen.core._vv and IOLOG.debug('%r.start_receive(%r)', self, stream) + side = stream.receive_side + assert side and side.fd is not None + if side.fd not in self._reader_by_fd: + self._reader_by_fd[side.fd] = side + self._control(side, select.KQ_FILTER_READ, select.KQ_EV_ADD) + + def stop_receive(self, stream): + mitogen.core._vv and IOLOG.debug('%r.stop_receive(%r)', self, stream) + side = stream.receive_side + if side.fd in self._reader_by_fd: + del self._reader_by_fd[stream.receive_side.fd] + self._control(side, select.KQ_FILTER_READ, select.KQ_EV_DELETE) + + def start_transmit(self, stream): + mitogen.core._vv and IOLOG.debug('%r.start_transmit(%r)', self, stream) + side = stream.transmit_side + assert side and side.fd is not None + if side.fd not in self._writer_by_fd: + self._writer_by_fd[side.fd] = side + self._control(side, select.KQ_FILTER_WRITE, select.KQ_EV_ADD) + + def stop_transmit(self, stream): + mitogen.core._vv and IOLOG.debug('%r.stop_transmit(%r)', self, stream) + side = stream.transmit_side + if side.fd in self._writer_by_fd: + del self._writer_by_fd[side.fd] + self._control(side, select.KQ_FILTER_WRITE, select.KQ_EV_DELETE) + + def poll(self, broker, timeout=None): + changelist = self._changelist + self._changelist = [] + for event in self._kqueue.control(changelist, 32, timeout): + if event.filter == select.KQ_FILTER_READ: + side = self._reader_by_fd.get(event.ident) + # Events can still be read for an already-discarded fd. + if side: + mitogen.core._vv and IOLOG.debug('%r: POLLIN: %r', self, side) + self._call(broker, side.stream, side.stream.on_receive) + elif event.filter == select.KQ_FILTER_WRITE: + side = self._writer_by_fd.get(event.ident) + if side: + mitogen.core._vv and IOLOG.debug('%r: POLLOUT: %r', self, side) + self._call(broker, side.stream, side.stream.on_transmit) + + + +POLLER_BY_SYSNAME = { + 'Darwin': KqueuePoller, + 'FreeBSD': KqueuePoller, + #'Linux': EpollPoller, +} +PREFERRED_POLLER = POLLER_BY_SYSNAME.get(os.uname()[0], mitogen.core.Poller) + + class TtyLogStream(mitogen.core.BasicStream): """ For "hybrid TTY/socketpair" mode, after a connection has been setup, a From 7320c542df0467e70136495926de639d86b43884 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Mon, 14 May 2018 17:52:08 +0000 Subject: [PATCH 13/59] issue #249: EpollPoller() for Linux. --- mitogen/parent.py | 83 +++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 81 insertions(+), 2 deletions(-) diff --git a/mitogen/parent.py b/mitogen/parent.py index 835c7e79..d9b67ede 100644 --- a/mitogen/parent.py +++ b/mitogen/parent.py @@ -542,7 +542,7 @@ class KqueuePoller(Poller): mitogen.core._vv and IOLOG.debug('%r.stop_receive(%r)', self, stream) side = stream.receive_side if side.fd in self._reader_by_fd: - del self._reader_by_fd[stream.receive_side.fd] + del self._reader_by_fd[side.fd] self._control(side, select.KQ_FILTER_READ, select.KQ_EV_DELETE) def start_transmit(self, stream): @@ -577,11 +577,90 @@ class KqueuePoller(Poller): self._call(broker, side.stream, side.stream.on_transmit) +class EpollPoller(Poller): + _repr = 'EpollPoller()' + + def __init__(self): + self._epoll = select.epoll() + self._registered_fds = set() + self._reader_by_fd = {} + self._writer_by_fd = {} + + @property + def readers(self): + return list(self._reader_by_fd.values()) + + @property + def writers(self): + return list(self._writer_by_fd.values()) + + def _control(self, fd): + mitogen.core._vv and IOLOG.debug('%r._control(%r)', self, fd) + mask = (((fd in self._reader_by_fd) and select.EPOLLIN) | + ((fd in self._writer_by_fd) and select.EPOLLOUT)) + if mask: + if fd in self._registered_fds: + self._epoll.modify(fd, mask) + else: + self._epoll.register(fd, mask) + self._registered_fds.add(fd) + elif fd in self._registered_fds: + self._epoll.unregister(fd) + self._registered_fds.remove(fd) + + def start_receive(self, stream): + mitogen.core._vv and IOLOG.debug('%r.start_receive(%r)', self, stream) + side = stream.receive_side + assert side and side.fd is not None + if side.fd not in self._reader_by_fd: + self._reader_by_fd[side.fd] = side + self._control(side.fd) + + def stop_receive(self, stream): + mitogen.core._vv and IOLOG.debug('%r.stop_receive(%r)', self, stream) + side = stream.receive_side + if side.fd in self._reader_by_fd: + del self._reader_by_fd[side.fd] + self._control(side.fd) + + def start_transmit(self, stream): + mitogen.core._vv and IOLOG.debug('%r.start_transmit(%r)', self, stream) + side = stream.transmit_side + assert side and side.fd is not None + if side.fd not in self._writer_by_fd: + self._writer_by_fd[side.fd] = side + self._control(side.fd) + + def stop_transmit(self, stream): + mitogen.core._vv and IOLOG.debug('%r.stop_transmit(%r)', self, stream) + side = stream.transmit_side + if side.fd in self._writer_by_fd: + del self._writer_by_fd[side.fd] + self._control(side.fd) + + def poll(self, broker, timeout=None): + the_timeout = -1 + if timeout is not None: + the_timeout = timeout + + for fd, ev in self._epoll.poll(the_timeout): + if ev & select.EPOLLIN: + side = self._reader_by_fd.get(fd) + # Events can still be read for an already-discarded fd. + if side: + mitogen.core._vv and IOLOG.debug('%r: POLLIN: %r', self, side) + self._call(broker, side.stream, side.stream.on_receive) + elif ev & select.EPOLLOUT: + side = self._writer_by_fd.get(fd) + if side: + mitogen.core._vv and IOLOG.debug('%r: POLLOUT: %r', self, side) + self._call(broker, side.stream, side.stream.on_transmit) + POLLER_BY_SYSNAME = { 'Darwin': KqueuePoller, 'FreeBSD': KqueuePoller, - #'Linux': EpollPoller, + 'Linux': EpollPoller, } PREFERRED_POLLER = POLLER_BY_SYSNAME.get(os.uname()[0], mitogen.core.Poller) From 11c2e4ab3e26a2f51e095d9c58938b8a1440a79c Mon Sep 17 00:00:00 2001 From: David Wilson Date: Mon, 14 May 2018 22:58:56 +0100 Subject: [PATCH 14/59] core: set _v and _vv to True in enable_debug_logging(). router.enable_debug() has been broken for ages. --- mitogen/core.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/mitogen/core.py b/mitogen/core.py index cfea37d6..64363951 100644 --- a/mitogen/core.py +++ b/mitogen/core.py @@ -273,6 +273,9 @@ class PidfulStreamHandler(logging.StreamHandler): def enable_debug_logging(): + global _v, _vv + _v = True + _vv = True root = logging.getLogger() root.setLevel(logging.DEBUG) IOLOG.setLevel(logging.DEBUG) From 9abcf63155e1b67ace570d6e9f5e98bb72aa2788 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Mon, 14 May 2018 23:32:26 +0100 Subject: [PATCH 15/59] issue #249: Poller API v2 (BSD only). Now it's BasicStream/Side-agnostic, so it can be reused for Latch and iter_read(). --- mitogen/core.py | 135 +++++++++++++++++++++++----------------------- mitogen/parent.py | 85 ++++++++++++++--------------- 2 files changed, 108 insertions(+), 112 deletions(-) diff --git a/mitogen/core.py b/mitogen/core.py index 64363951..b952e601 100644 --- a/mitogen/core.py +++ b/mitogen/core.py @@ -762,6 +762,7 @@ class Side(object): def __init__(self, stream, fd, cloexec=True, keep_alive=True): self.stream = stream self.fd = fd + self.closed = False self.keep_alive = keep_alive self._fork_refs[id(self)] = self if cloexec: @@ -771,21 +772,16 @@ class Side(object): def __repr__(self): return '' % (self.stream, self.fd) - def fileno(self): - if self.fd is None: - raise StreamError('%r.fileno() called but no FD set', self) - return self.fd - @classmethod def _on_fork(cls): for side in list(cls._fork_refs.values()): side.close() def close(self): - if self.fd is not None: + if not self.closed: _vv and IOLOG.debug('%r.close()', self) os.close(self.fd) - self.fd = None + self.closed = True def read(self, n=CHUNK_SIZE): s, disconnected = io_op(os.read, self.fd, n) @@ -1442,66 +1438,51 @@ class Router(object): class Poller(object): def __init__(self): - self.readers = [] - self.writers = [] + self._rfds = {} + self._wfds = {} _repr = 'Poller()' - def __repr__(self): - return self._repr - - def _list_discard(self, lst, value): - try: - lst.remove(value) - except ValueError: - pass + @property + def readers(self): + return list(self._rfds.items()) - def _list_add(self, lst, value): - if value not in lst: - lst.append(value) + @property + def writers(self): + return list(self._wfds.items()) - def start_receive(self, stream): - _vv and IOLOG.debug('%r.start_receive(%r)', self, stream) - assert stream.receive_side and stream.receive_side.fd is not None - self._list_add(self.readers, stream.receive_side) + def __repr__(self): + return self._repr - def stop_receive(self, stream): - _vv and IOLOG.debug('%r.stop_receive(%r)', self, stream) - self._list_discard(self.readers, stream.receive_side) + def start_receive(self, fd, data=None): + self._rfds[fd] = data or fd - def start_transmit(self, stream): - _vv and IOLOG.debug('%r._start_transmit(%r)', self, stream) - assert stream.transmit_side and stream.transmit_side.fd is not None - self._list_add(self.writers, stream.transmit_side) + def stop_receive(self, fd): + self._rfds.pop(fd, None) - def stop_transmit(self, stream): - _vv and IOLOG.debug('%r._stop_transmit(%r)', self, stream) - self._list_discard(self.writers, stream.transmit_side) + def start_transmit(self, fd, data=None): + self._wfds[fd] = data or fd - def _call(self, broker, stream, func): - try: - func(broker) - except Exception: - LOG.exception('%r crashed', stream) - stream.on_disconnect(self) + def stop_transmit(self, fd): + self._wfds.pop(fd, None) - def poll(self, broker, timeout=None): + def poll(self, timeout=None): _vv and IOLOG.debug('%r.poll(%r)', self, timeout) - #IOLOG.debug('readers = %r', self.readers) - #IOLOG.debug('writers = %r', self.writers) - (rsides, wsides, _), _ = io_op(select.select, - self.readers, - self.writers, + IOLOG.debug('readers = %r', self._rfds) + IOLOG.debug('writers = %r', self._wfds) + (rfds, wfds, _), _ = io_op(select.select, + self._rfds, + self._wfds, (), timeout ) - for side in rsides: - _vv and IOLOG.debug('%r: POLLIN for %r', self, side) - self._call(broker, side.stream, side.stream.on_receive) + for fd in rfds: + _vv and IOLOG.debug('%r: POLLIN for %r', self, fd) + yield self._rfds[fd] - for side in wsides: - _vv and IOLOG.debug('%r: POLLOUT for %r', self, side) - self._call(broker, side.stream, side.stream.on_transmit) + for fd in wfds: + _vv and IOLOG.debug('%r: POLLOUT for %r', self, fd) + yield self._wfds[fd] class Broker(object): @@ -1515,9 +1496,10 @@ class Broker(object): self._waker = Waker(self) self.defer = self._waker.defer self.poller = self.poller_class() - self.poller.start_receive(self._waker) - self.readers = [self._waker.receive_side] - self.writers = [] + self.poller.start_receive( + self._waker.receive_side.fd, + (self._waker.receive_side, self._waker.on_receive) + ) self._thread = threading.Thread( target=_profile_hook, args=('broker', self._broker_main), @@ -1527,33 +1509,54 @@ class Broker(object): self._waker.broker_ident = self._thread.ident def start_receive(self, stream): - self.defer(self.poller.start_receive, stream) + _vv and IOLOG.debug('%r.start_receive(%r)', self, stream) + side = stream.receive_side + assert side and side.fd is not None + self.defer(self.poller.start_receive, + side.fd, (side, stream.on_receive)) def stop_receive(self, stream): - self.defer(self.poller.stop_receive, stream) + _vv and IOLOG.debug('%r.stop_receive(%r)', self, stream) + self.defer(self.poller.stop_receive, stream.receive_side.fd) def _start_transmit(self, stream): - self.poller.start_transmit(stream) + _vv and IOLOG.debug('%r._start_transmit(%r)', self, stream) + side = stream.transmit_side + assert side and side.fd is not None + self.poller.start_transmit(side.fd, (side, stream.on_transmit)) def _stop_transmit(self, stream): - self.poller.stop_transmit(stream) + _vv and IOLOG.debug('%r._stop_transmit(%r)', self, stream) + self.poller.stop_transmit(stream.transmit_side.fd) def keep_alive(self): - return sum((side.keep_alive for side in self.poller.readers), 0) + it = (side.keep_alive for (_, (side, _)) in self.poller.readers) + return sum(it, 0) + + def _call(self, stream, func): + try: + func(self) + except Exception: + LOG.exception('%r crashed', stream) + stream.on_disconnect(self) + + def _loop_once(self, timeout=None): + _vv and IOLOG.debug('%r._loop_once(%r)', self, timeout) + for (side, func) in self.poller.poll(timeout): + self._call(side.stream, func) def _broker_main(self): try: while self._alive: - self.poller.poll(self) + self._loop_once() fire(self, 'shutdown') - - for side in set(self.poller.readers).union(self.poller.writers): - self.poller._call(self, side.stream, side.stream.on_shutdown) + for _, (side, _) in self.poller.readers + self.poller.writers: + self._call(side.stream, side.stream.on_shutdown) deadline = time.time() + self.shutdown_timeout while self.keep_alive() and time.time() < deadline: - self.poller.poll(self, max(0, deadline - time.time())) + self._loop_once(max(0, deadline - time.time())) if self.keep_alive(): LOG.error('%r: some streams did not close gracefully. ' @@ -1561,7 +1564,7 @@ class Broker(object): 'more child processes still connected to ' 'our stdout/stderr pipes.', self) - for side in set(self.readers).union(self.writers): + for _, (side, _) in self.poller.readers + self.poller.writers: LOG.error('_broker_main() force disconnecting %r', side) side.stream.on_disconnect(self) except Exception: diff --git a/mitogen/parent.py b/mitogen/parent.py index d9b67ede..89a7cf8b 100644 --- a/mitogen/parent.py +++ b/mitogen/parent.py @@ -513,68 +513,61 @@ class KqueuePoller(Poller): def __init__(self): self._kqueue = select.kqueue() - self._reader_by_fd = {} - self._writer_by_fd = {} + self._rfds = {} + self._wfds = {} self._changelist = [] @property def readers(self): - return list(self._reader_by_fd.values()) + return list(self._rfds.items()) @property def writers(self): - return list(self._writer_by_fd.values()) + return list(self._wfds.items()) - def _control(self, side, filters, flags): + def _control(self, fd, filters, flags): mitogen.core._vv and IOLOG.debug( - '%r._control(%r, %r, %r)', self, side, filters, flags) - self._changelist.append(select.kevent(side.fd, filters, flags)) - - def start_receive(self, stream): - mitogen.core._vv and IOLOG.debug('%r.start_receive(%r)', self, stream) - side = stream.receive_side - assert side and side.fd is not None - if side.fd not in self._reader_by_fd: - self._reader_by_fd[side.fd] = side - self._control(side, select.KQ_FILTER_READ, select.KQ_EV_ADD) - - def stop_receive(self, stream): - mitogen.core._vv and IOLOG.debug('%r.stop_receive(%r)', self, stream) - side = stream.receive_side - if side.fd in self._reader_by_fd: - del self._reader_by_fd[side.fd] - self._control(side, select.KQ_FILTER_READ, select.KQ_EV_DELETE) - - def start_transmit(self, stream): - mitogen.core._vv and IOLOG.debug('%r.start_transmit(%r)', self, stream) - side = stream.transmit_side - assert side and side.fd is not None - if side.fd not in self._writer_by_fd: - self._writer_by_fd[side.fd] = side - self._control(side, select.KQ_FILTER_WRITE, select.KQ_EV_ADD) - - def stop_transmit(self, stream): - mitogen.core._vv and IOLOG.debug('%r.stop_transmit(%r)', self, stream) - side = stream.transmit_side - if side.fd in self._writer_by_fd: - del self._writer_by_fd[side.fd] - self._control(side, select.KQ_FILTER_WRITE, select.KQ_EV_DELETE) + '%r._control(%r, %r, %r)', self, fd, filters, flags) + self._changelist.append(select.kevent(fd, filters, flags)) + + def start_receive(self, fd, data=None): + mitogen.core._vv and IOLOG.debug('%r.start_receive(%r, %d)', + self, fd, data) + if fd not in self._rfds: + self._control(fd, select.KQ_FILTER_READ, select.KQ_EV_ADD) + self._rfds[fd] = data or fd + + def stop_receive(self, fd): + mitogen.core._vv and IOLOG.debug('%r.stop_receive(%r)', self, fd, data) + if fd in self._rfds: + self._control(fd, select.KQ_FILTER_READ, select.KQ_EV_DELETE) + del self._rfds[fd] + + def start_transmit(self, fd, data=None): + mitogen.core._vv and IOLOG.debug('%r.start_transmit(%r)', self, fd, data) + if fd not in self._wfds: + self._control(fd, select.KQ_FILTER_WRITE, select.KQ_EV_ADD) + self._wfds[fd] = data or fd + + def stop_transmit(self, fd): + mitogen.core._vv and IOLOG.debug('%r.stop_transmit(%r)', self, fd) + if fd in self._wfds: + self._control(fd, select.KQ_FILTER_WRITE, select.KQ_EV_DELETE) + del self._wfds[fd] def poll(self, broker, timeout=None): changelist = self._changelist self._changelist = [] for event in self._kqueue.control(changelist, 32, timeout): if event.filter == select.KQ_FILTER_READ: - side = self._reader_by_fd.get(event.ident) - # Events can still be read for an already-discarded fd. - if side: + if event.ident in self._rfds: + # Events can still be read for an already-discarded fd. mitogen.core._vv and IOLOG.debug('%r: POLLIN: %r', self, side) - self._call(broker, side.stream, side.stream.on_receive) + yield self._rfds[event.ident] elif event.filter == select.KQ_FILTER_WRITE: - side = self._writer_by_fd.get(event.ident) - if side: + if event.ident in self._wfds: mitogen.core._vv and IOLOG.debug('%r: POLLOUT: %r', self, side) - self._call(broker, side.stream, side.stream.on_transmit) + yield self._wfds[event.ident] class EpollPoller(Poller): @@ -660,7 +653,7 @@ class EpollPoller(Poller): POLLER_BY_SYSNAME = { 'Darwin': KqueuePoller, 'FreeBSD': KqueuePoller, - 'Linux': EpollPoller, + #'Linux': EpollPoller, } PREFERRED_POLLER = POLLER_BY_SYSNAME.get(os.uname()[0], mitogen.core.Poller) @@ -760,7 +753,7 @@ class Stream(mitogen.core.Stream): def on_shutdown(self, broker): """Request the slave gracefully shut itself down.""" LOG.debug('%r closing CALL_FUNCTION channel', self) - self.send( + self._send( mitogen.core.Message( src_id=mitogen.context_id, dst_id=self.remote_id, From b6124f83965e3f3fd2fff63643c92f75202de902 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Mon, 14 May 2018 22:50:35 +0000 Subject: [PATCH 16/59] issue #249: EpollPoller v2. --- mitogen/parent.py | 105 ++++++++++++++++++++-------------------------- 1 file changed, 45 insertions(+), 60 deletions(-) diff --git a/mitogen/parent.py b/mitogen/parent.py index 89a7cf8b..5c815fe9 100644 --- a/mitogen/parent.py +++ b/mitogen/parent.py @@ -497,7 +497,6 @@ class Argv(object): return ' '.join(map(self.escape, self.argv)) - class Poller(mitogen.core.Poller): @classmethod def from_existing(cls, poller): @@ -538,7 +537,7 @@ class KqueuePoller(Poller): self._rfds[fd] = data or fd def stop_receive(self, fd): - mitogen.core._vv and IOLOG.debug('%r.stop_receive(%r)', self, fd, data) + mitogen.core._vv and IOLOG.debug('%r.stop_receive(%r)', self, fd) if fd in self._rfds: self._control(fd, select.KQ_FILTER_READ, select.KQ_EV_DELETE) del self._rfds[fd] @@ -559,15 +558,14 @@ class KqueuePoller(Poller): changelist = self._changelist self._changelist = [] for event in self._kqueue.control(changelist, 32, timeout): - if event.filter == select.KQ_FILTER_READ: - if event.ident in self._rfds: - # Events can still be read for an already-discarded fd. - mitogen.core._vv and IOLOG.debug('%r: POLLIN: %r', self, side) - yield self._rfds[event.ident] - elif event.filter == select.KQ_FILTER_WRITE: - if event.ident in self._wfds: - mitogen.core._vv and IOLOG.debug('%r: POLLOUT: %r', self, side) - yield self._wfds[event.ident] + fd = event.ident + if event.filter == select.KQ_FILTER_READ and fd in self._rfds: + # Events can still be read for an already-discarded fd. + mitogen.core._vv and IOLOG.debug('%r: POLLIN: %r', self, fd) + yield self._rfds[fd] + elif event.filter == select.KQ_FILTER_WRITE and fd in self._wfds: + mitogen.core._vv and IOLOG.debug('%r: POLLOUT: %r', self, fd) + yield self._wfds[fd] class EpollPoller(Poller): @@ -576,21 +574,21 @@ class EpollPoller(Poller): def __init__(self): self._epoll = select.epoll() self._registered_fds = set() - self._reader_by_fd = {} - self._writer_by_fd = {} + self._rfds = {} + self._wfds = {} @property def readers(self): - return list(self._reader_by_fd.values()) + return list(self._rfds.items()) @property def writers(self): - return list(self._writer_by_fd.values()) + return list(self._wfds.items()) def _control(self, fd): mitogen.core._vv and IOLOG.debug('%r._control(%r)', self, fd) - mask = (((fd in self._reader_by_fd) and select.EPOLLIN) | - ((fd in self._writer_by_fd) and select.EPOLLOUT)) + mask = (((fd in self._rfds) and select.EPOLLIN) | + ((fd in self._wfds) and select.EPOLLOUT)) if mask: if fd in self._registered_fds: self._epoll.modify(fd, mask) @@ -601,59 +599,47 @@ class EpollPoller(Poller): self._epoll.unregister(fd) self._registered_fds.remove(fd) - def start_receive(self, stream): - mitogen.core._vv and IOLOG.debug('%r.start_receive(%r)', self, stream) - side = stream.receive_side - assert side and side.fd is not None - if side.fd not in self._reader_by_fd: - self._reader_by_fd[side.fd] = side - self._control(side.fd) - - def stop_receive(self, stream): - mitogen.core._vv and IOLOG.debug('%r.stop_receive(%r)', self, stream) - side = stream.receive_side - if side.fd in self._reader_by_fd: - del self._reader_by_fd[side.fd] - self._control(side.fd) - - def start_transmit(self, stream): - mitogen.core._vv and IOLOG.debug('%r.start_transmit(%r)', self, stream) - side = stream.transmit_side - assert side and side.fd is not None - if side.fd not in self._writer_by_fd: - self._writer_by_fd[side.fd] = side - self._control(side.fd) - - def stop_transmit(self, stream): - mitogen.core._vv and IOLOG.debug('%r.stop_transmit(%r)', self, stream) - side = stream.transmit_side - if side.fd in self._writer_by_fd: - del self._writer_by_fd[side.fd] - self._control(side.fd) + def start_receive(self, fd, data=None): + mitogen.core._vv and IOLOG.debug('%r.start_receive(%r, %r)', + self, fd, data) + self._rfds[fd] = data or fd + self._control(fd) + + def stop_receive(self, fd): + mitogen.core._vv and IOLOG.debug('%r.stop_receive(%r)', self, fd) + self._rfds.pop(fd, None) + self._control(fd) + + def start_transmit(self, fd, data=None): + mitogen.core._vv and IOLOG.debug('%r.start_transmit(%r, %r)', + self, fd, data) + self._wfds[fd] = data or fd + self._control(fd) + + def stop_transmit(self, fd): + mitogen.core._vv and IOLOG.debug('%r.stop_transmit(%r)', self, fd) + self._wfds.pop(fd, None) + self._control(fd) def poll(self, broker, timeout=None): the_timeout = -1 if timeout is not None: the_timeout = timeout - for fd, ev in self._epoll.poll(the_timeout): - if ev & select.EPOLLIN: - side = self._reader_by_fd.get(fd) + for fd, event in self._epoll.poll(the_timeout): + if event & select.EPOLLIN and fd in self._rfds: # Events can still be read for an already-discarded fd. - if side: - mitogen.core._vv and IOLOG.debug('%r: POLLIN: %r', self, side) - self._call(broker, side.stream, side.stream.on_receive) - elif ev & select.EPOLLOUT: - side = self._writer_by_fd.get(fd) - if side: - mitogen.core._vv and IOLOG.debug('%r: POLLOUT: %r', self, side) - self._call(broker, side.stream, side.stream.on_transmit) + mitogen.core._vv and IOLOG.debug('%r: POLLIN: %r', self, fd) + yield self._rfds[fd] + elif event & select.EPOLLOUT and fd in self._wfds: + mitogen.core._vv and IOLOG.debug('%r: POLLOUT: %r', self, fd) + yield self._wfds[fd] POLLER_BY_SYSNAME = { 'Darwin': KqueuePoller, 'FreeBSD': KqueuePoller, - #'Linux': EpollPoller, + 'Linux': EpollPoller, } PREFERRED_POLLER = POLLER_BY_SYSNAME.get(os.uname()[0], mitogen.core.Poller) @@ -1030,7 +1016,7 @@ class RouteMonitor(object): def _on_stream_disconnect(self, stream): """ - Respond to disconnection of a local stream by + Respond to disconnection of a local stream by """ LOG.debug('%r is gone; propagating DEL_ROUTE for %r', stream, stream.routes) @@ -1172,7 +1158,6 @@ class Router(mitogen.core.Router): try: stream.connect() except mitogen.core.TimeoutError: - e = sys.exc_info()[1] raise mitogen.core.StreamError(self.connection_timeout_msg) context.name = stream.name self.route_monitor.notice_stream(stream) From 9905f6d8b4efef574cf1dedff05f7a0c0b8afbf6 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Mon, 14 May 2018 23:56:11 +0100 Subject: [PATCH 17/59] issue #249: make write_all() and iter_read() use PREFERRED_POLLER. --- mitogen/parent.py | 37 +++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/mitogen/parent.py b/mitogen/parent.py index 5c815fe9..ca77df8b 100644 --- a/mitogen/parent.py +++ b/mitogen/parent.py @@ -356,6 +356,8 @@ def hybrid_tty_create_child(args): def write_all(fd, s, deadline=None): + poller = PREFERRED_POLLER() + poller.start_transmit(fd) timeout = None written = 0 @@ -365,33 +367,28 @@ def write_all(fd, s, deadline=None): if timeout == 0: raise mitogen.core.TimeoutError('write timed out') - _, wfds, _ = select.select([], [fd], [], timeout) - if not wfds: - continue - - n, disconnected = mitogen.core.io_op(os.write, fd, buffer(s, written)) - if disconnected: - raise mitogen.core.StreamError('EOF on stream during write') + for fd in poller.poll(timeout): + n, disconnected = mitogen.core.io_op(os.write, fd, buffer(s, written)) + if disconnected: + raise mitogen.core.StreamError('EOF on stream during write') written += n def iter_read(fds, deadline=None): - fds = list(fds) + poller = PREFERRED_POLLER() + for fd in fds: + poller.start_receive(fd) + bits = [] timeout = None - - while fds: + while poller.readers: if deadline is not None: timeout = max(0, deadline - time.time()) if timeout == 0: break - rfds, _, _ = select.select(fds, [], [], timeout) - if not rfds: - continue - - for fd in rfds: + for fd in poller.poll(timeout): s, disconnected = mitogen.core.io_op(os.read, fd, 4096) if disconnected or not s: IOLOG.debug('iter_read(%r) -> disconnected', fd) @@ -530,7 +527,7 @@ class KqueuePoller(Poller): self._changelist.append(select.kevent(fd, filters, flags)) def start_receive(self, fd, data=None): - mitogen.core._vv and IOLOG.debug('%r.start_receive(%r, %d)', + mitogen.core._vv and IOLOG.debug('%r.start_receive(%r, %r)', self, fd, data) if fd not in self._rfds: self._control(fd, select.KQ_FILTER_READ, select.KQ_EV_ADD) @@ -543,7 +540,8 @@ class KqueuePoller(Poller): del self._rfds[fd] def start_transmit(self, fd, data=None): - mitogen.core._vv and IOLOG.debug('%r.start_transmit(%r)', self, fd, data) + mitogen.core._vv and IOLOG.debug('%r.start_transmit(%r, %r)', + self, fd, data) if fd not in self._wfds: self._control(fd, select.KQ_FILTER_WRITE, select.KQ_EV_ADD) self._wfds[fd] = data or fd @@ -641,7 +639,10 @@ POLLER_BY_SYSNAME = { 'FreeBSD': KqueuePoller, 'Linux': EpollPoller, } -PREFERRED_POLLER = POLLER_BY_SYSNAME.get(os.uname()[0], mitogen.core.Poller) +PREFERRED_POLLER = POLLER_BY_SYSNAME.get( + os.uname()[0], + mitogen.core.Poller, +) class TtyLogStream(mitogen.core.BasicStream): From 4df020827da16eaa70b0a036019afaf94a5cc37e Mon Sep 17 00:00:00 2001 From: David Wilson Date: Tue, 15 May 2018 00:01:01 +0100 Subject: [PATCH 18/59] issue #249: explicitly close pollers when done. --- mitogen/core.py | 3 ++ mitogen/parent.py | 72 +++++++++++++++++++++++++++-------------------- 2 files changed, 45 insertions(+), 30 deletions(-) diff --git a/mitogen/core.py b/mitogen/core.py index b952e601..189e6d31 100644 --- a/mitogen/core.py +++ b/mitogen/core.py @@ -1454,6 +1454,9 @@ class Poller(object): def __repr__(self): return self._repr + def close(self): + pass + def start_receive(self, fd, data=None): self._rfds[fd] = data or fd diff --git a/mitogen/parent.py b/mitogen/parent.py index ca77df8b..d40bfee8 100644 --- a/mitogen/parent.py +++ b/mitogen/parent.py @@ -356,23 +356,27 @@ def hybrid_tty_create_child(args): def write_all(fd, s, deadline=None): - poller = PREFERRED_POLLER() - poller.start_transmit(fd) timeout = None written = 0 + poller = PREFERRED_POLLER() + poller.start_transmit(fd) - while written < len(s): - if deadline is not None: - timeout = max(0, deadline - time.time()) - if timeout == 0: - raise mitogen.core.TimeoutError('write timed out') + try: + while written < len(s): + if deadline is not None: + timeout = max(0, deadline - time.time()) + if timeout == 0: + raise mitogen.core.TimeoutError('write timed out') - for fd in poller.poll(timeout): - n, disconnected = mitogen.core.io_op(os.write, fd, buffer(s, written)) - if disconnected: - raise mitogen.core.StreamError('EOF on stream during write') + for fd in poller.poll(timeout): + n, disconnected = mitogen.core.io_op( + os.write, fd, buffer(s, written)) + if disconnected: + raise mitogen.core.StreamError('EOF on stream during write') - written += n + written += n + finally: + poller.close() def iter_read(fds, deadline=None): @@ -382,28 +386,30 @@ def iter_read(fds, deadline=None): bits = [] timeout = None - while poller.readers: - if deadline is not None: - timeout = max(0, deadline - time.time()) - if timeout == 0: - break - - for fd in poller.poll(timeout): - s, disconnected = mitogen.core.io_op(os.read, fd, 4096) - if disconnected or not s: - IOLOG.debug('iter_read(%r) -> disconnected', fd) - fds.remove(fd) - else: - IOLOG.debug('iter_read(%r) -> %r', fd, s) - bits.append(s) - yield s - - if not fds: + try: + while poller.readers: + if deadline is not None: + timeout = max(0, deadline - time.time()) + if timeout == 0: + break + + for fd in poller.poll(timeout): + s, disconnected = mitogen.core.io_op(os.read, fd, 4096) + if disconnected or not s: + IOLOG.debug('iter_read(%r) -> disconnected', fd) + fds.remove(fd) + else: + IOLOG.debug('iter_read(%r) -> %r', fd, s) + bits.append(s) + yield s + finally: + poller.close() + + if not poller.readers: raise mitogen.core.StreamError( 'EOF on stream; last 300 bytes received: %r' % (''.join(bits)[-300:],) ) - raise mitogen.core.TimeoutError('read timed out') @@ -513,6 +519,9 @@ class KqueuePoller(Poller): self._wfds = {} self._changelist = [] + def close(self): + self._kqueue.close() + @property def readers(self): return list(self._rfds.items()) @@ -575,6 +584,9 @@ class EpollPoller(Poller): self._rfds = {} self._wfds = {} + def close(self): + self._epoll.close() + @property def readers(self): return list(self._rfds.items()) From 5645629e5d200f60a81010ae038c64a6b63a33b7 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Tue, 15 May 2018 00:06:31 +0100 Subject: [PATCH 19/59] issue #249: the new pollers must handle syscall restarts too. --- mitogen/parent.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/mitogen/parent.py b/mitogen/parent.py index d40bfee8..23f55817 100644 --- a/mitogen/parent.py +++ b/mitogen/parent.py @@ -564,7 +564,9 @@ class KqueuePoller(Poller): def poll(self, broker, timeout=None): changelist = self._changelist self._changelist = [] - for event in self._kqueue.control(changelist, 32, timeout): + events, _ = mitogen.core.io_op(self._kqueue.control, + changelist, 32, timeout) + for event in events: fd = event.ident if event.filter == select.KQ_FILTER_READ and fd in self._rfds: # Events can still be read for an already-discarded fd. @@ -636,7 +638,8 @@ class EpollPoller(Poller): if timeout is not None: the_timeout = timeout - for fd, event in self._epoll.poll(the_timeout): + events, _ = mitogen.core.io_op(self._epoll.poll, the_timeout) + for fd, event in events: if event & select.EPOLLIN and fd in self._rfds: # Events can still be read for an already-discarded fd. mitogen.core._vv and IOLOG.debug('%r: POLLIN: %r', self, fd) From aa8f786413126e4e296245ae660c7c5de91ba5a2 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Tue, 15 May 2018 00:11:08 +0100 Subject: [PATCH 20/59] issue #249: fix Poller.from_existing() for v2 API --- mitogen/parent.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/mitogen/parent.py b/mitogen/parent.py index 23f55817..ebe61fc1 100644 --- a/mitogen/parent.py +++ b/mitogen/parent.py @@ -504,10 +504,10 @@ class Poller(mitogen.core.Poller): @classmethod def from_existing(cls, poller): self = cls() - for reader in poller.readers: - self.start_receive(reader.stream) - for writer in poller.writers: - self.start_transmit(writer.stream) + for fd, data in poller.readers: + self.start_receive(fd, data) + for fd, data in poller.writers: + self.start_transmit(fd, data) class KqueuePoller(Poller): From dcf0aa351eb4548095bb7b509ae9eff0897e2404 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Tue, 15 May 2018 00:15:32 +0100 Subject: [PATCH 21/59] issue #249: whoops, fix new poller timeouts. --- mitogen/core.py | 1 - mitogen/parent.py | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/mitogen/core.py b/mitogen/core.py index 189e6d31..428965eb 100644 --- a/mitogen/core.py +++ b/mitogen/core.py @@ -1063,7 +1063,6 @@ class Latch(object): def get(self, timeout=None, block=True): _vv and IOLOG.debug('%r.get(timeout=%r, block=%r)', self, timeout, block) - self._lock.acquire() try: if self.closed: diff --git a/mitogen/parent.py b/mitogen/parent.py index ebe61fc1..01b558fe 100644 --- a/mitogen/parent.py +++ b/mitogen/parent.py @@ -561,7 +561,7 @@ class KqueuePoller(Poller): self._control(fd, select.KQ_FILTER_WRITE, select.KQ_EV_DELETE) del self._wfds[fd] - def poll(self, broker, timeout=None): + def poll(self, timeout=None): changelist = self._changelist self._changelist = [] events, _ = mitogen.core.io_op(self._kqueue.control, @@ -633,7 +633,7 @@ class EpollPoller(Poller): self._wfds.pop(fd, None) self._control(fd) - def poll(self, broker, timeout=None): + def poll(self, timeout=None): the_timeout = -1 if timeout is not None: the_timeout = timeout From 1070dfae721751eb585a555342e30ee077d8a07c Mon Sep 17 00:00:00 2001 From: David Wilson Date: Tue, 15 May 2018 00:17:38 +0100 Subject: [PATCH 22/59] issue #249: fix iter_read() regression. --- mitogen/parent.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mitogen/parent.py b/mitogen/parent.py index 01b558fe..9e40d17f 100644 --- a/mitogen/parent.py +++ b/mitogen/parent.py @@ -397,7 +397,7 @@ def iter_read(fds, deadline=None): s, disconnected = mitogen.core.io_op(os.read, fd, 4096) if disconnected or not s: IOLOG.debug('iter_read(%r) -> disconnected', fd) - fds.remove(fd) + poller.stop_receive(fd) else: IOLOG.debug('iter_read(%r) -> %r', fd, s) bits.append(s) From 36a102486184cba0708f5400ea56f1b91cb56a1f Mon Sep 17 00:00:00 2001 From: David Wilson Date: Tue, 15 May 2018 00:21:02 +0100 Subject: [PATCH 23/59] issue #249: port Latch to poller too. This is probably going to suck for perf :/ --- mitogen/core.py | 116 ++++++++++++++++++++++++---------------------- mitogen/parent.py | 6 +++ 2 files changed, 67 insertions(+), 55 deletions(-) diff --git a/mitogen/core.py b/mitogen/core.py index 428965eb..f58eac83 100644 --- a/mitogen/core.py +++ b/mitogen/core.py @@ -1019,7 +1019,60 @@ def _unpickle_context(router, context_id, name): return router.context_class(router, context_id, name) +class Poller(object): + def __init__(self): + self._rfds = {} + self._wfds = {} + + _repr = 'Poller()' + + @property + def readers(self): + return list(self._rfds.items()) + + @property + def writers(self): + return list(self._wfds.items()) + + def __repr__(self): + return self._repr + + def close(self): + pass + + def start_receive(self, fd, data=None): + self._rfds[fd] = data or fd + + def stop_receive(self, fd): + self._rfds.pop(fd, None) + + def start_transmit(self, fd, data=None): + self._wfds[fd] = data or fd + + def stop_transmit(self, fd): + self._wfds.pop(fd, None) + + def poll(self, timeout=None): + _vv and IOLOG.debug('%r.poll(%r)', self, timeout) + IOLOG.debug('readers = %r', self._rfds) + IOLOG.debug('writers = %r', self._wfds) + (rfds, wfds, _), _ = io_op(select.select, + self._rfds, + self._wfds, + (), timeout + ) + + for fd in rfds: + _vv and IOLOG.debug('%r: POLLIN for %r', self, fd) + yield self._rfds[fd] + + for fd in wfds: + _vv and IOLOG.debug('%r: POLLOUT for %r', self, fd) + yield self._wfds[fd] + + class Latch(object): + poller_class = Poller closed = False _waking = 0 _sockets = [] @@ -1084,10 +1137,15 @@ class Latch(object): _vv and IOLOG.debug('%r._get_sleep(timeout=%r, block=%r)', self, timeout, block) e = None + poller = self.poller_class() + poller.start_receive(rsock.fileno()) try: - io_op(select.select, [rsock], [], [], timeout) - except Exception: - e = sys.exc_info()[1] + try: + list(poller.poll(timeout)) + except Exception: + e = sys.exc_info()[1] + finally: + poller.close() self._lock.acquire() try: @@ -1435,58 +1493,6 @@ class Router(object): self.broker.defer(self._async_route, msg) -class Poller(object): - def __init__(self): - self._rfds = {} - self._wfds = {} - - _repr = 'Poller()' - - @property - def readers(self): - return list(self._rfds.items()) - - @property - def writers(self): - return list(self._wfds.items()) - - def __repr__(self): - return self._repr - - def close(self): - pass - - def start_receive(self, fd, data=None): - self._rfds[fd] = data or fd - - def stop_receive(self, fd): - self._rfds.pop(fd, None) - - def start_transmit(self, fd, data=None): - self._wfds[fd] = data or fd - - def stop_transmit(self, fd): - self._wfds.pop(fd, None) - - def poll(self, timeout=None): - _vv and IOLOG.debug('%r.poll(%r)', self, timeout) - IOLOG.debug('readers = %r', self._rfds) - IOLOG.debug('writers = %r', self._wfds) - (rfds, wfds, _), _ = io_op(select.select, - self._rfds, - self._wfds, - (), timeout - ) - - for fd in rfds: - _vv and IOLOG.debug('%r: POLLIN for %r', self, fd) - yield self._rfds[fd] - - for fd in wfds: - _vv and IOLOG.debug('%r: POLLOUT for %r', self, fd) - yield self._wfds[fd] - - class Broker(object): poller_class = Poller _waker = None diff --git a/mitogen/parent.py b/mitogen/parent.py index 9e40d17f..3cfff8fb 100644 --- a/mitogen/parent.py +++ b/mitogen/parent.py @@ -654,11 +654,17 @@ POLLER_BY_SYSNAME = { 'FreeBSD': KqueuePoller, 'Linux': EpollPoller, } + PREFERRED_POLLER = POLLER_BY_SYSNAME.get( os.uname()[0], mitogen.core.Poller, ) +# For apps that start threads dynamically, it's possible Latch will also get +# very high-numbered wait fds when there are many connections, and so select() +# becomes useless there too. So swap in our favourite poller. +mitogen.core.Latch.poller_class = PREFERRED_POLLER + class TtyLogStream(mitogen.core.BasicStream): """ From 4cd9e09130f217565e437a917feb4e8902c4a08d Mon Sep 17 00:00:00 2001 From: David Wilson Date: Tue, 15 May 2018 00:22:02 +0100 Subject: [PATCH 24/59] issue #249: docs: remove limitation --- docs/ansible.rst | 4 ---- 1 file changed, 4 deletions(-) diff --git a/docs/ansible.rst b/docs/ansible.rst index 74f95679..c59486e7 100644 --- a/docs/ansible.rst +++ b/docs/ansible.rst @@ -137,10 +137,6 @@ Noteworthy Differences * Asynchronous jobs presently exist only for the duration of a run, and time limits are not implemented. -* Due to use of :func:`select.select` the IO multiplexer breaks down around 100 - targets, expect performance degradation as this number is approached and - errant behaviour as it is exceeded. A replacement will appear soon. - * The undocumented ability to extend and override :mod:`ansible.module_utils` by supplying a ``module_utils`` directory alongside a custom new-style module is not yet supported. From 07056b0dd16f89c31bbd4f7a2b643683b50f9062 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Tue, 15 May 2018 00:27:41 +0100 Subject: [PATCH 25/59] issue #249: fix ordering bug masked by previous implementation --- mitogen/core.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mitogen/core.py b/mitogen/core.py index f58eac83..30d2c911 100644 --- a/mitogen/core.py +++ b/mitogen/core.py @@ -805,11 +805,11 @@ class BasicStream(object): def on_disconnect(self, broker): LOG.debug('%r.on_disconnect()', self) - broker.stop_receive(self) - broker._stop_transmit(self) if self.receive_side: + broker.stop_receive(self) self.receive_side.close() if self.transmit_side: + broker._stop_transmit(self) self.transmit_side.close() fire(self, 'disconnect') From 6b9881804602256d08f5b938d3746fa2a5a8c735 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Tue, 15 May 2018 01:32:46 +0100 Subject: [PATCH 26/59] issue #249: epoll distinguishes between hangup and disconnect ..typical Linux, for certain descriptor types only. So our receive mask must match both, and normalize it into a read event like every other poller. --- mitogen/parent.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/mitogen/parent.py b/mitogen/parent.py index 3cfff8fb..693a82b7 100644 --- a/mitogen/parent.py +++ b/mitogen/parent.py @@ -633,6 +633,9 @@ class EpollPoller(Poller): self._wfds.pop(fd, None) self._control(fd) + _inmask = (getattr(select, 'EPOLLIN', 0) | + getattr(select, 'EPOLLHUP', 0)) + def poll(self, timeout=None): the_timeout = -1 if timeout is not None: @@ -640,7 +643,7 @@ class EpollPoller(Poller): events, _ = mitogen.core.io_op(self._epoll.poll, the_timeout) for fd, event in events: - if event & select.EPOLLIN and fd in self._rfds: + if event & self._inmask and fd in self._rfds: # Events can still be read for an already-discarded fd. mitogen.core._vv and IOLOG.debug('%r: POLLIN: %r', self, fd) yield self._rfds[fd] From 6d18ce81d8808425e34e94370a93bcce0c0c1640 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Tue, 15 May 2018 01:49:03 +0100 Subject: [PATCH 27/59] issue #249: restore duplex behaviour for epoll With epoll() there is only one kernel-side object per file descriptor, which is why _control() is such a pain. Since we merge receive/transmit watching into that single object, we must always test the mask for both conditions when reading results. Kqueue isn't/doesn't appear to be like this. The identity of a Kqueue event is keyed on (fd, filter), and we register a separate event for both transmit and receive, so the 'elif' in KqueuePoller.poll() does not appear to need to change. Previously, a FD marked for read+write would not indicate writeability until it was no longer readable. --- mitogen/parent.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mitogen/parent.py b/mitogen/parent.py index 693a82b7..089d8603 100644 --- a/mitogen/parent.py +++ b/mitogen/parent.py @@ -647,7 +647,7 @@ class EpollPoller(Poller): # Events can still be read for an already-discarded fd. mitogen.core._vv and IOLOG.debug('%r: POLLIN: %r', self, fd) yield self._rfds[fd] - elif event & select.EPOLLOUT and fd in self._wfds: + if event & select.EPOLLOUT and fd in self._wfds: mitogen.core._vv and IOLOG.debug('%r: POLLOUT: %r', self, fd) yield self._wfds[fd] From 70376d861a22f43427c0923d1f76f2cc3f67723d Mon Sep 17 00:00:00 2001 From: David Wilson Date: Tue, 15 May 2018 01:57:09 +0100 Subject: [PATCH 28/59] issue #217: docs: remove limitation --- docs/ansible.rst | 4 ---- 1 file changed, 4 deletions(-) diff --git a/docs/ansible.rst b/docs/ansible.rst index c59486e7..c401dd42 100644 --- a/docs/ansible.rst +++ b/docs/ansible.rst @@ -137,10 +137,6 @@ Noteworthy Differences * Asynchronous jobs presently exist only for the duration of a run, and time limits are not implemented. -* The undocumented ability to extend and override :mod:`ansible.module_utils` - by supplying a ``module_utils`` directory alongside a custom new-style module - is not yet supported. - * "Module Replacer" style modules are not supported. These rarely appear in practice, and light web searches failed to reveal many examples of them. From 5bdc1719c5cb19d044ef4e057176cf13352535a3 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Tue, 15 May 2018 04:15:32 +0100 Subject: [PATCH 29/59] issue #249: epoll() raises IOError for EINTR, not select.error. --- mitogen/core.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mitogen/core.py b/mitogen/core.py index 30d2c911..9fc3b82f 100644 --- a/mitogen/core.py +++ b/mitogen/core.py @@ -235,7 +235,7 @@ def io_op(func, *args): while True: try: return func(*args), False - except (select.error, OSError): + except (select.error, OSError, IOError): e = sys.exc_info()[1] _vv and IOLOG.debug('io_op(%r) -> OSError: %s', func, e) if e[0] == errno.EINTR: From 05a5f2b6e5841210f4a6e2540071e1205fc67b98 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Tue, 15 May 2018 04:18:55 +0100 Subject: [PATCH 30/59] core: if Poller.poll() fails, TimeoutError would be raised. We must check whether poller threw an exception both in the case that we weren't woken and the case that we were. --- mitogen/core.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mitogen/core.py b/mitogen/core.py index 9fc3b82f..09e42b34 100644 --- a/mitogen/core.py +++ b/mitogen/core.py @@ -1153,7 +1153,7 @@ class Latch(object): del self._sleeping[i] self._sockets.append((rsock, wsock)) if i >= self._waking: - raise TimeoutError() + raise e or TimeoutError() self._waking -= 1 if rsock.recv(2) != '\x7f': raise LatchError('internal error: received >1 wakeups') From 55fff54774147e2112581eb52815098dd6574240 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Tue, 15 May 2018 09:22:35 +0100 Subject: [PATCH 31/59] core: make try/catch logic a little clearer in Latch.get() --- mitogen/core.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/mitogen/core.py b/mitogen/core.py index 09e42b34..c5c31360 100644 --- a/mitogen/core.py +++ b/mitogen/core.py @@ -1131,22 +1131,22 @@ class Latch(object): finally: self._lock.release() - return self._get_sleep(timeout, block, rsock, wsock) - - def _get_sleep(self, timeout, block, rsock, wsock): - _vv and IOLOG.debug('%r._get_sleep(timeout=%r, block=%r)', - self, timeout, block) - e = None poller = self.poller_class() poller.start_receive(rsock.fileno()) try: - try: - list(poller.poll(timeout)) - except Exception: - e = sys.exc_info()[1] + return self._get_sleep(poller, timeout, block, rsock, wsock) finally: poller.close() + def _get_sleep(self, poller, timeout, block, rsock, wsock): + _vv and IOLOG.debug('%r._get_sleep(timeout=%r, block=%r)', + self, timeout, block) + e = None + try: + list(poller.poll(timeout)) + except Exception: + e = sys.exc_info()[1] + self._lock.acquire() try: i = self._sleeping.index(wsock) From 7d0209d8decbee244c926cbffaf274c323816066 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Tue, 15 May 2018 09:28:03 +0100 Subject: [PATCH 32/59] issue #249: have upgrade_router() upgrade the poller too. Now when a child becomes a parent, it gets a new poller suitable for many more children than was possible using select(). --- mitogen/parent.py | 51 ++++++++++++++++++++++++++++++++++------------- 1 file changed, 37 insertions(+), 14 deletions(-) diff --git a/mitogen/parent.py b/mitogen/parent.py index 089d8603..692adf24 100644 --- a/mitogen/parent.py +++ b/mitogen/parent.py @@ -422,8 +422,41 @@ def discard_until(fd, s, deadline): return +def _upgrade_broker(broker): + """ + Extract the poller state from Broker and replace it with the industrial + strength poller for this OS. Must run on the Broker thread. + """ + # This function is deadly! The act of calling start_receive() generates log + # messages which must be silenced as the upgrade progresses, otherwise the + # poller state will change as it is copied, resulting in write fds that are + # lost. (Due to LogHandler->Router->Stream->Broker->Poller, where Stream + # only calls start_transmit() when transitioning from empty to non-empty + # buffer. If the start_transmit() is lost, writes from the child hang + # permanently). + root = logging.getLogger() + old_level = root.level + root.setLevel(logging.CRITICAL) + + old = broker.poller + new = PREFERRED_POLLER() + for fd, data in old.readers: + new.start_receive(fd, data) + for fd, data in old.writers: + new.start_transmit(fd, data) + + old.close() + broker.poller = new + root.setLevel(old_level) + LOG.debug('replaced %r with %r (new: %d readers, %d writers; ' + 'old: %d readers, %d writers)', old, new, + len(new.readers), len(new.writers), + len(old.readers), len(old.writers)) + + def upgrade_router(econtext): if not isinstance(econtext.router, Router): # TODO + econtext.broker.defer(_upgrade_broker, econtext.broker) econtext.router.__class__ = Router # TODO econtext.router.upgrade( importer=econtext.importer, @@ -500,17 +533,7 @@ class Argv(object): return ' '.join(map(self.escape, self.argv)) -class Poller(mitogen.core.Poller): - @classmethod - def from_existing(cls, poller): - self = cls() - for fd, data in poller.readers: - self.start_receive(fd, data) - for fd, data in poller.writers: - self.start_transmit(fd, data) - - -class KqueuePoller(Poller): +class KqueuePoller(mitogen.core.Poller): _repr = 'KqueuePoller()' def __init__(self): @@ -577,11 +600,11 @@ class KqueuePoller(Poller): yield self._wfds[fd] -class EpollPoller(Poller): +class EpollPoller(mitogen.core.Poller): _repr = 'EpollPoller()' def __init__(self): - self._epoll = select.epoll() + self._epoll = select.epoll(32) self._registered_fds = set() self._rfds = {} self._wfds = {} @@ -641,7 +664,7 @@ class EpollPoller(Poller): if timeout is not None: the_timeout = timeout - events, _ = mitogen.core.io_op(self._epoll.poll, the_timeout) + events, _ = mitogen.core.io_op(self._epoll.poll, the_timeout, 32) for fd, event in events: if event & self._inmask and fd in self._rfds: # Events can still be read for an already-discarded fd. From 2c58591129c836b5ef7192a49243dc55bece1721 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Tue, 15 May 2018 21:13:38 +0100 Subject: [PATCH 33/59] Bump Ansible version in dev_requirements.txt --- dev_requirements.txt | 2 +- tests/ansible/gcloud-ansible-playbook.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/dev_requirements.txt b/dev_requirements.txt index 59bf43de..4ae7df8e 100644 --- a/dev_requirements.txt +++ b/dev_requirements.txt @@ -1,5 +1,5 @@ -r docs/docs-requirements.txt -ansible==2.3.1.0 +ansible==2.5.2 coverage==4.5.1 Django==1.6.11; python_version < '2.7' Django==1.11.5; python_version >= '2.7' # for module_finder_test diff --git a/tests/ansible/gcloud-ansible-playbook.py b/tests/ansible/gcloud-ansible-playbook.py index 10ba97a9..7c221110 100755 --- a/tests/ansible/gcloud-ansible-playbook.py +++ b/tests/ansible/gcloud-ansible-playbook.py @@ -8,7 +8,7 @@ import googleapiclient.discovery def main(): project = 'mitogen-load-testing' - zone = 'asia-south1-c' + zone = 'europe-west1-d' group_name = 'micro-debian9' client = googleapiclient.discovery.build('compute', 'v1') From 1eb5c20f57ffbce74eaaffa319bc4797f06cda4f Mon Sep 17 00:00:00 2001 From: David Wilson Date: Tue, 15 May 2018 21:29:00 +0100 Subject: [PATCH 34/59] ansible: add dummy init.pys so setup.py find_packages() DTRT. --- ansible_mitogen/plugins/__init__.py | 0 ansible_mitogen/plugins/connection/__init__.py | 0 ansible_mitogen/plugins/strategy/__init__.py | 0 3 files changed, 0 insertions(+), 0 deletions(-) create mode 100644 ansible_mitogen/plugins/__init__.py create mode 100644 ansible_mitogen/plugins/connection/__init__.py create mode 100644 ansible_mitogen/plugins/strategy/__init__.py diff --git a/ansible_mitogen/plugins/__init__.py b/ansible_mitogen/plugins/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/ansible_mitogen/plugins/connection/__init__.py b/ansible_mitogen/plugins/connection/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/ansible_mitogen/plugins/strategy/__init__.py b/ansible_mitogen/plugins/strategy/__init__.py new file mode 100644 index 00000000..e69de29b From a3995f8e5f9245c84195b35186eea2f5f6a7482e Mon Sep 17 00:00:00 2001 From: David Wilson Date: Tue, 15 May 2018 21:34:13 +0100 Subject: [PATCH 35/59] ansible: remove hard-coded dw username. --- tests/ansible/gcloud-ansible-playbook.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/ansible/gcloud-ansible-playbook.py b/tests/ansible/gcloud-ansible-playbook.py index 7c221110..77dd6d99 100755 --- a/tests/ansible/gcloud-ansible-playbook.py +++ b/tests/ansible/gcloud-ansible-playbook.py @@ -27,7 +27,6 @@ def main(): print 'Addresses:', ips os.execvp('ansible-playbook', [ 'anisble-playbook', - '--user=dw', '--inventory-file=' + ','.join(ips) + ',' ] + sys.argv[1:]) From dbcee4041a50b4868436c9682f70a6d240ff5b1e Mon Sep 17 00:00:00 2001 From: David Wilson Date: Tue, 15 May 2018 22:10:36 +0100 Subject: [PATCH 36/59] tests: refactor gcloud.py to be dynamic inventory. --- tests/ansible/ansible.cfg | 2 +- .../inventory/gcloud.py} | 16 +++++++++++----- 2 files changed, 12 insertions(+), 6 deletions(-) rename tests/ansible/{gcloud-ansible-playbook.py => lib/inventory/gcloud.py} (72%) diff --git a/tests/ansible/ansible.cfg b/tests/ansible/ansible.cfg index 8c9f66e0..437e67b7 100644 --- a/tests/ansible/ansible.cfg +++ b/tests/ansible/ansible.cfg @@ -1,5 +1,5 @@ [defaults] -inventory = hosts +inventory = hosts,lib/inventory gathering = explicit strategy_plugins = ../../ansible_mitogen/plugins/strategy action_plugins = lib/action diff --git a/tests/ansible/gcloud-ansible-playbook.py b/tests/ansible/lib/inventory/gcloud.py similarity index 72% rename from tests/ansible/gcloud-ansible-playbook.py rename to tests/ansible/lib/inventory/gcloud.py index 77dd6d99..b537ae74 100755 --- a/tests/ansible/gcloud-ansible-playbook.py +++ b/tests/ansible/lib/inventory/gcloud.py @@ -1,8 +1,13 @@ #!/usr/bin/env python +import json import os import sys +if not os.environ.get('MITOGEN_GCLOUD_GROUP'): + sys.stdout.write('{}') + sys.exit(0) + import googleapiclient.discovery @@ -24,11 +29,12 @@ def main(): #for config in interface['accessConfigs'] ) - print 'Addresses:', ips - os.execvp('ansible-playbook', [ - 'anisble-playbook', - '--inventory-file=' + ','.join(ips) + ',' - ] + sys.argv[1:]) + sys.stderr.write('Addresses: %s\n' % (ips,)) + sys.stdout.write(json.dumps({ + os.environ['MITOGEN_GCLOUD_GROUP']: { + 'hosts': ips + } + }, indent=4)) if __name__ == '__main__': From 1a0e630a80fbe29ce1937add949306cad92c164b Mon Sep 17 00:00:00 2001 From: David Wilson Date: Tue, 15 May 2018 22:15:49 +0100 Subject: [PATCH 37/59] tests: add debops to requirements --- dev_requirements.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/dev_requirements.txt b/dev_requirements.txt index 4ae7df8e..c8b6af13 100644 --- a/dev_requirements.txt +++ b/dev_requirements.txt @@ -3,6 +3,7 @@ ansible==2.5.2 coverage==4.5.1 Django==1.6.11; python_version < '2.7' Django==1.11.5; python_version >= '2.7' # for module_finder_test +debops==0.7.2 https://github.com/docker/docker-py/archive/1.10.6.tar.gz; python_version < '2.7' docker[tls]==2.5.1; python_version >= '2.7' mock==2.0.0 From a99c7a12f953675bf7dac59b5350a590f7642d9c Mon Sep 17 00:00:00 2001 From: David Wilson Date: Tue, 15 May 2018 23:40:06 +0000 Subject: [PATCH 38/59] tests: output split groups in gcloud.py --- tests/ansible/lib/inventory/gcloud.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/tests/ansible/lib/inventory/gcloud.py b/tests/ansible/lib/inventory/gcloud.py index b537ae74..956e955b 100755 --- a/tests/ansible/lib/inventory/gcloud.py +++ b/tests/ansible/lib/inventory/gcloud.py @@ -14,7 +14,7 @@ import googleapiclient.discovery def main(): project = 'mitogen-load-testing' zone = 'europe-west1-d' - group_name = 'micro-debian9' + group_name = 'target' client = googleapiclient.discovery.build('compute', 'v1') resp = client.instances().list(project=project, zone=zone).execute() @@ -30,11 +30,19 @@ def main(): ) sys.stderr.write('Addresses: %s\n' % (ips,)) - sys.stdout.write(json.dumps({ - os.environ['MITOGEN_GCLOUD_GROUP']: { + gname = os.environ['MITOGEN_GCLOUD_GROUP'] + groups = { + gname: { 'hosts': ips } - }, indent=4)) + } + + for i in 1, 10, 20, 50, 100: + groups['%s-%s' % (gname, i)] = { + 'hosts': ips[:i] + } + + sys.stdout.write(json.dumps(groups, indent=4)) if __name__ == '__main__': From b0aa4131739b2e80662749bc47134516bfc7dd33 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Thu, 17 May 2018 18:14:20 +0100 Subject: [PATCH 39/59] tests: import benchmark reproduction setup playbook Incomplete, also needs to mess around with Gcloud routing and handle box setup/teardown, because that's another "well engineered" waste of time. --- tests/ansible/gcloud/README.md | 6 ++ tests/ansible/gcloud/ansible.cfg | 3 + tests/ansible/gcloud/controller.yml | 86 +++++++++++++++++++++++++++++ tests/ansible/gcloud/hosts | 2 + 4 files changed, 97 insertions(+) create mode 100644 tests/ansible/gcloud/README.md create mode 100644 tests/ansible/gcloud/ansible.cfg create mode 100644 tests/ansible/gcloud/controller.yml create mode 100644 tests/ansible/gcloud/hosts diff --git a/tests/ansible/gcloud/README.md b/tests/ansible/gcloud/README.md new file mode 100644 index 00000000..9ec53050 --- /dev/null +++ b/tests/ansible/gcloud/README.md @@ -0,0 +1,6 @@ + +# Command line. + +```` +time LANG=C LC_ALL=C ANSIBLE_STRATEGY=mitogen MITOGEN_GCLOUD_GROUP=debops_all_hosts debops common +``` diff --git a/tests/ansible/gcloud/ansible.cfg b/tests/ansible/gcloud/ansible.cfg new file mode 100644 index 00000000..d1fcd982 --- /dev/null +++ b/tests/ansible/gcloud/ansible.cfg @@ -0,0 +1,3 @@ +[defaults] +inventory = hosts +retry_files_enabled = False diff --git a/tests/ansible/gcloud/controller.yml b/tests/ansible/gcloud/controller.yml new file mode 100644 index 00000000..4c768510 --- /dev/null +++ b/tests/ansible/gcloud/controller.yml @@ -0,0 +1,86 @@ + +- hosts: controller + tasks: + - shell: "rsync -a ~/.ssh {{inventory_hostname}}:" + connection: local + + - lineinfile: + line: "net.ipv4.ip_forward=1" + path: /etc/sysctl.conf + register: sysctl_conf + become: true + + - shell: /sbin/sysctl -p + when: sysctl_conf.changed + become: true + + - shell: | + iptables -t nat -F; + iptables -t nat -X; + iptables -t nat -A POSTROUTING -j MASQUERADE; + become: true + + - apt: name={{item}} state=installed + become: true + with_items: + - python-pip + - python-virtualenv + - strace + - libldap2-dev + - libsasl2-dev + - build-essential + - git + + - git: + dest: ~/mitogen + repo: https://github.com/dw/mitogen.git + version: dmw + + - git: + dest: ~/ansible + repo: https://github.com/dw/ansible.git + version: lazy-vars + + - pip: + virtualenv: ~/venv + requirements: ~/mitogen/dev_requirements.txt + + - pip: + virtualenv: ~/venv + editable: true + name: ~/mitogen + + - pip: + virtualenv: ~/venv + editable: true + name: ~/ansible + + - lineinfile: + line: "source $HOME/venv/bin/activate" + path: ~/.profile + + - name: debops-init + shell: ~/venv/bin/debops-init ~/prj + args: + creates: ~/prj + + - name: grpvars + copy: + dest: "{{ansible_user_dir}}/prj/ansible/inventory/group_vars/all/dhparam.yml" + content: | + --- + dhparam__bits: [ '256' ] + + - blockinfile: + path: ~/prj/.debops.cfg + insertafter: '\[ansible defaults\]' + block: | + strategy_plugins = {{ansible_user_dir}}/mitogen/ansible_mitogen/plugins/strategy + forks = 50 + host_key_checking = False + + - file: + path: ~/prj/ansible/inventory/gcloud.py + state: link + src: ~/mitogen/tests/ansible/lib/inventory/gcloud.py + diff --git a/tests/ansible/gcloud/hosts b/tests/ansible/gcloud/hosts new file mode 100644 index 00000000..b4562cb5 --- /dev/null +++ b/tests/ansible/gcloud/hosts @@ -0,0 +1,2 @@ +[controller] +35.206.145.240 From 863a95e8605dfd8e7c92455cab8121039cf7f691 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Thu, 17 May 2018 22:23:56 +0100 Subject: [PATCH 40/59] docs: update contributors --- docs/contributors.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/contributors.rst b/docs/contributors.rst index 39d545ff..d0e1929d 100644 --- a/docs/contributors.rst +++ b/docs/contributors.rst @@ -29,8 +29,8 @@ sponsorship and outstanding future-thinking of its early adopters.


- For global career opportunities, please visit http://www.cgi.com/en/careers/working-at-cgi. + For career opportunities, please visit cgi-group.co.uk/defence-and-intelligence-opportunities.

From eac4cc7afeca05edb2957d0e79069d922edca738 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Fri, 18 May 2018 10:57:44 +0100 Subject: [PATCH 41/59] tests: speed things up --- tests/ansible/lib/inventory/gcloud.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/ansible/lib/inventory/gcloud.py b/tests/ansible/lib/inventory/gcloud.py index 956e955b..2135d913 100755 --- a/tests/ansible/lib/inventory/gcloud.py +++ b/tests/ansible/lib/inventory/gcloud.py @@ -4,7 +4,7 @@ import json import os import sys -if not os.environ.get('MITOGEN_GCLOUD_GROUP'): +if (not os.environ.get('MITOGEN_GCLOUD_GROUP')) or any('--host' in s for s in sys.argv): sys.stdout.write('{}') sys.exit(0) From 00edf0d66d04f637f26f843d5cb54a12934b252e Mon Sep 17 00:00:00 2001 From: David Wilson Date: Wed, 23 May 2018 16:23:22 +0100 Subject: [PATCH 42/59] core: have ExternalContext accept a config dict rather than kwargs. The parameter lists had gotten out of control. --- mitogen/core.py | 78 ++++++++++++++++++++++++-------------------- mitogen/fakessh.py | 39 ++++++++++++---------- mitogen/fork.py | 10 +++--- mitogen/parent.py | 6 ++-- tests/parent_test.py | 1 + 5 files changed, 74 insertions(+), 60 deletions(-) diff --git a/mitogen/core.py b/mitogen/core.py index c5c31360..ad948853 100644 --- a/mitogen/core.py +++ b/mitogen/core.py @@ -1596,11 +1596,14 @@ class Broker(object): class ExternalContext(object): detached = False + def __init__(self, config): + self.config = config + def _on_broker_shutdown(self): self.channel.close() def _on_broker_exit(self): - if not self.profiling: + if not self.config['profiling']: os.kill(os.getpid(), signal.SIGTERM) def _on_shutdown_msg(self, msg): @@ -1638,29 +1641,31 @@ class ExternalContext(object): LOG.error('Stream had %d bytes after 2000ms', pending) self.broker.defer(stream.on_disconnect, self.broker) - def _setup_master(self, max_message_size, profiling, unidirectional, - parent_id, context_id, in_fd, out_fd): - Router.max_message_size = max_message_size - self.profiling = profiling - if profiling: + def _setup_master(self): + Router.max_message_size = self.config['max_message_size'] + if self.config['profiling']: enable_profiling() self.broker = Broker() self.router = Router(self.broker) - self.router.undirectional = unidirectional + self.router.undirectional = self.config['unidirectional'] self.router.add_handler( fn=self._on_shutdown_msg, handle=SHUTDOWN, policy=has_parent_authority, ) self.master = Context(self.router, 0, 'master') + parent_id = self.config['parent_ids'][0] if parent_id == 0: self.parent = self.master else: self.parent = Context(self.router, parent_id, 'parent') - self.channel = Receiver(router=self.router, - handle=CALL_FUNCTION, - policy=has_parent_authority) + in_fd = self.config.get('in_fd', 100) + out_fd = self.config.get('out_fd', 1) + + self.recv = Receiver(router=self.router, + handle=CALL_FUNCTION, + policy=has_parent_authority) self.stream = Stream(self.router, parent_id) self.stream.name = 'parent' self.stream.accept(in_fd, out_fd) @@ -1678,20 +1683,22 @@ class ExternalContext(object): except OSError: pass # No first stage exists (e.g. fakessh) - def _setup_logging(self, debug, log_level): + def _setup_logging(self): root = logging.getLogger() - root.setLevel(log_level) + root.setLevel(self.config['log_level']) root.handlers = [LogHandler(self.master)] - if debug: + if self.config['debug']: enable_debug_logging() - def _setup_importer(self, importer, core_src_fd, whitelist, blacklist): + def _setup_importer(self): + importer = self.config.get('importer') if importer: importer._install_handler(self.router) importer._context = self.parent else: + core_src_fd = self.config.get('core_src_fd', 101) if core_src_fd: - fp = os.fdopen(101, 'r', 1) + fp = os.fdopen(core_src_fd, 'r', 1) try: core_size = int(fp.readline()) core_src = fp.read(core_size) @@ -1702,8 +1709,13 @@ class ExternalContext(object): else: core_src = None - importer = Importer(self.router, self.parent, - core_src, whitelist, blacklist) + importer = Importer( + self.router, + self.parent, + core_src, + self.config.get('whitelist', ()), + self.config.get('blacklist', ()), + ) self.importer = importer self.router.importer = importer @@ -1723,12 +1735,12 @@ class ExternalContext(object): sys.modules['mitogen.core'] = mitogen.core del sys.modules['__main__'] - def _setup_globals(self, version, context_id, parent_ids): - mitogen.__version__ = version + def _setup_globals(self): mitogen.is_master = False - mitogen.context_id = context_id - mitogen.parent_ids = parent_ids - mitogen.parent_id = parent_ids[0] + mitogen.__version__ = self.config['version'] + mitogen.context_id = self.config['context_id'] + mitogen.parent_ids = self.config['parent_ids'][:] + mitogen.parent_id = mitogen.parent_ids[0] def _setup_stdio(self): # We must open this prior to closing stdout, otherwise it will recycle @@ -1774,7 +1786,7 @@ class ExternalContext(object): return fn(*args, **kwargs) def _dispatch_calls(self): - for msg in self.channel: + for msg in self.recv: try: msg.reply(self._dispatch_one(msg)) except Exception: @@ -1783,28 +1795,24 @@ class ExternalContext(object): msg.reply(CallError(e)) self.dispatch_stopped = True - def main(self, parent_ids, context_id, debug, profiling, log_level, - unidirectional, max_message_size, version, in_fd=100, out_fd=1, - core_src_fd=101, setup_stdio=True, setup_package=True, - importer=None, whitelist=(), blacklist=()): - self._setup_master(max_message_size, profiling, unidirectional, - parent_ids[0], context_id, in_fd, out_fd) + def main(self): + self._setup_master() try: try: - self._setup_logging(debug, log_level) - self._setup_importer(importer, core_src_fd, whitelist, blacklist) + self._setup_logging() + self._setup_importer() self._reap_first_stage() - if setup_package: + if self.config.get('setup_package', True): self._setup_package() - self._setup_globals(version, context_id, parent_ids) - if setup_stdio: + self._setup_globals() + if self.config.get('setup_stdio', True): self._setup_stdio() self.router.register(self.parent, self.stream) sys.executable = os.environ.pop('ARGV0', sys.executable) _v and LOG.debug('Connected to %s; my ID is %r, PID is %r', - self.parent, context_id, os.getpid()) + self.parent, mitogen.context_id, os.getpid()) _v and LOG.debug('Recovered sys.executable: %r', sys.executable) _profile_hook('main', self._dispatch_calls) diff --git a/mitogen/fakessh.py b/mitogen/fakessh.py index 3ee91015..27cb4acd 100644 --- a/mitogen/fakessh.py +++ b/mitogen/fakessh.py @@ -304,6 +304,25 @@ def _fakessh_main(dest_context_id, econtext): process.control.put(('exit', None)) +def _get_econtext_config(context, sock2): + parent_ids = mitogen.parent_ids[:] + parent_ids.insert(0, mitogen.context_id) + return { + 'context_id': context.context_id, + 'core_src_fd': None, + 'debug': getattr(context.router, 'debug', False), + 'in_fd': sock2.fileno(), + 'log_level': mitogen.parent.get_log_level(), + 'max_message_size': context.router.max_message_size, + 'out_fd': sock2.fileno(), + 'parent_ids': parent_ids, + 'profiling': getattr(context.router, 'profiling', False), + 'unidirectional': getattr(context.router, 'unidirectional', False), + 'setup_stdio': False, + 'version': mitogen.__version__, + } + + # # Public API. # @@ -328,9 +347,6 @@ def run(dest, router, args, deadline=None, econtext=None): # Held in socket buffer until process is booted. fakessh.call_async(_fakessh_main, dest.context_id) - parent_ids = mitogen.parent_ids[:] - parent_ids.insert(0, mitogen.context_id) - tmp_path = tempfile.mkdtemp(prefix='mitogen_fakessh') try: ssh_path = os.path.join(tmp_path, 'ssh') @@ -339,20 +355,9 @@ def run(dest, router, args, deadline=None, econtext=None): fp.write('#!%s\n' % (sys.executable,)) fp.write(inspect.getsource(mitogen.core)) fp.write('\n') - fp.write('ExternalContext().main(**%r)\n' % ({ - 'context_id': context_id, - 'core_src_fd': None, - 'debug': getattr(router, 'debug', False), - 'in_fd': sock2.fileno(), - 'log_level': mitogen.parent.get_log_level(), - 'max_message_size': router.max_message_size, - 'out_fd': sock2.fileno(), - 'parent_ids': parent_ids, - 'profiling': getattr(router, 'profiling', False), - 'unidirectional': getattr(router, 'unidirectional', False), - 'setup_stdio': False, - 'version': mitogen.__version__, - },)) + fp.write('ExternalContext(%r).main()\n' % ( + _get_econtext_config(context, sock2), + )) finally: fp.close() diff --git a/mitogen/fork.py b/mitogen/fork.py index 4a5627dc..dc013fa5 100644 --- a/mitogen/fork.py +++ b/mitogen/fork.py @@ -148,12 +148,12 @@ class Stream(mitogen.parent.Stream): os.close(devnull) childfp.close() - kwargs = self.get_main_kwargs() - kwargs['core_src_fd'] = None - kwargs['importer'] = self.importer - kwargs['setup_package'] = False + config = self.get_econtext_config() + config['core_src_fd'] = None + config['importer'] = self.importer + config['setup_package'] = False try: - mitogen.core.ExternalContext().main(**kwargs) + mitogen.core.ExternalContext(config).main() finally: # Don't trigger atexit handlers, they were copied from the parent. os._exit(0) diff --git a/mitogen/parent.py b/mitogen/parent.py index 692adf24..9d66c736 100644 --- a/mitogen/parent.py +++ b/mitogen/parent.py @@ -896,7 +896,7 @@ class Stream(mitogen.core.Stream): 'exec(_(_("%s".encode(),"base64"),"zip"))' % (encoded,) ] - def get_main_kwargs(self): + def get_econtext_config(self): assert self.max_message_size is not None parent_ids = mitogen.parent_ids[:] parent_ids.insert(0, mitogen.context_id) @@ -915,8 +915,8 @@ class Stream(mitogen.core.Stream): def get_preamble(self): source = inspect.getsource(mitogen.core) - source += '\nExternalContext().main(**%r)\n' % ( - self.get_main_kwargs(), + source += '\nExternalContext(%r).main()\n' % ( + self.get_econtext_config(), ) return zlib.compress(minimize_source(source), 9) diff --git a/tests/parent_test.py b/tests/parent_test.py index 4cbdad38..c4dafb62 100644 --- a/tests/parent_test.py +++ b/tests/parent_test.py @@ -1,6 +1,7 @@ import errno import os import subprocess +import sys import tempfile import time From b0ce6eecd74c5b6bc749e7fd7bae649c62570670 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Wed, 23 May 2018 16:25:17 +0100 Subject: [PATCH 43/59] fork: support on_start= argument. --- docs/api.rst | 15 ++++++++------- mitogen/core.py | 3 +++ mitogen/fork.py | 6 +++++- tests/fork_test.py | 7 +++++++ 4 files changed, 23 insertions(+), 8 deletions(-) diff --git a/docs/api.rst b/docs/api.rst index 365edcb6..ecee398d 100644 --- a/docs/api.rst +++ b/docs/api.rst @@ -576,7 +576,7 @@ Router Class **Context Factories** - .. method:: fork (new_stack=False, on_fork=None, debug=False, profiling=False, via=None) + .. method:: fork (on_fork=None, on_start=None, debug=False, profiling=False, via=None) Construct a context on the local machine by forking the current process. The forked child receives a new identity, sets up a new broker @@ -650,18 +650,19 @@ Router Class The associated stream implementation is :py:class:`mitogen.fork.Stream`. - :param bool new_stack: - If :py:data:`True`, arrange for the local thread stack to be - discarded, by forking from a new thread. Aside from clean - tracebacks, this has the effect of causing objects referenced by - the stack to cease existing in the child. - :param function on_fork: Function invoked as `on_fork()` from within the child process. This permits supplying a program-specific cleanup function to break locks and close file descriptors belonging to the parent from within the child. + :param function on_start: + Invoked as `on_start(econtext)` from within the child process after + it has been set up, but before the function dispatch loop starts. + This permits supplying a custom child main function that inherits + rich data structures that cannot normally be passed via a + serialization. + :param Context via: Same as the `via` parameter for :py:meth:`local`. diff --git a/mitogen/core.py b/mitogen/core.py index ad948853..531ce0ee 100644 --- a/mitogen/core.py +++ b/mitogen/core.py @@ -1786,6 +1786,9 @@ class ExternalContext(object): return fn(*args, **kwargs) def _dispatch_calls(self): + if self.config.get('on_start'): + self.config['on_start'](self) + for msg in self.recv: try: msg.reply(self._dispatch_one(msg)) diff --git a/mitogen/fork.py b/mitogen/fork.py index dc013fa5..151ace9b 100644 --- a/mitogen/fork.py +++ b/mitogen/fork.py @@ -90,12 +90,14 @@ class Stream(mitogen.parent.Stream): on_fork = None def construct(self, old_router, max_message_size, on_fork=None, - debug=False, profiling=False, unidirectional=False): + debug=False, profiling=False, unidirectional=False, + on_start=None): # fork method only supports a tiny subset of options. super(Stream, self).construct(max_message_size=max_message_size, debug=debug, profiling=profiling, unidirectional=False) self.on_fork = on_fork + self.on_start = on_start responder = getattr(old_router, 'responder', None) if isinstance(responder, mitogen.parent.ModuleForwarder): @@ -152,6 +154,8 @@ class Stream(mitogen.parent.Stream): config['core_src_fd'] = None config['importer'] = self.importer config['setup_package'] = False + if self.on_start: + config['on_start'] = self.on_start try: mitogen.core.ExternalContext(config).main() finally: diff --git a/tests/fork_test.py b/tests/fork_test.py index c5e19adb..61c0e16d 100644 --- a/tests/fork_test.py +++ b/tests/fork_test.py @@ -70,6 +70,13 @@ class ForkTest(testlib.RouterMixin, unittest2.TestCase): context = self.router.fork() self.assertEqual(2, context.call(exercise_importer, 1)) + def test_on_start(self): + recv = mitogen.core.Receiver(self.router) + def on_start(econtext): + sender = mitogen.core.Sender(econtext.parent, recv.handle) + sender.send(123) + context = self.router.fork(on_start=on_start) + self.assertEquals(123, recv.get().unpickle()) class DoubleChildTest(testlib.RouterMixin, unittest2.TestCase): def test_okay(self): From 7a592d1c34a3d2c66660f4b70eb35d441a871a97 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Thu, 24 May 2018 23:30:56 +0100 Subject: [PATCH 44/59] core: better Poller.__repr__ --- mitogen/core.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/mitogen/core.py b/mitogen/core.py index 531ce0ee..80fb589a 100644 --- a/mitogen/core.py +++ b/mitogen/core.py @@ -1024,8 +1024,6 @@ class Poller(object): self._rfds = {} self._wfds = {} - _repr = 'Poller()' - @property def readers(self): return list(self._rfds.items()) @@ -1035,7 +1033,7 @@ class Poller(object): return list(self._wfds.items()) def __repr__(self): - return self._repr + return '%s(%#x)' % (type(self).__name__, id(self)) def close(self): pass From ddf28987a079587366fe2b11ebea7a2971032cce Mon Sep 17 00:00:00 2001 From: David Wilson Date: Thu, 24 May 2018 23:32:07 +0100 Subject: [PATCH 45/59] master: split Select() into new module to reduce wire size. service.py currently imports master.py(+parent.py) just to get Select(). --- ansible_mitogen/mixins.py | 6 +- mitogen/core.py | 5 +- mitogen/master.py | 104 ----------------------------- mitogen/parent.py | 4 +- mitogen/select.py | 133 ++++++++++++++++++++++++++++++++++++++ mitogen/service.py | 4 +- tests/select_test.py | 32 ++++----- 7 files changed, 161 insertions(+), 127 deletions(-) create mode 100644 mitogen/select.py diff --git a/ansible_mitogen/mixins.py b/ansible_mitogen/mixins.py index b9cbd3e7..fdd104b2 100644 --- a/ansible_mitogen/mixins.py +++ b/ansible_mitogen/mixins.py @@ -49,7 +49,7 @@ except ImportError: # Ansible<2.4 from ansible.plugins import module_loader import mitogen.core -import mitogen.master +import mitogen.select import mitogen.utils import ansible_mitogen.connection @@ -253,7 +253,7 @@ class ActionModuleMixin(ansible.plugins.action.ActionBase): """ LOG.debug('_remote_chmod(%r, mode=%r, sudoable=%r)', paths, mode, sudoable) - return self.fake_shell(lambda: mitogen.master.Select.all( + return self.fake_shell(lambda: mitogen.select.Select.all( self._connection.call_async( ansible_mitogen.target.set_file_mode, path, mode ) @@ -268,7 +268,7 @@ class ActionModuleMixin(ansible.plugins.action.ActionBase): LOG.debug('_remote_chown(%r, user=%r, sudoable=%r)', paths, user, sudoable) ent = self.call(pwd.getpwnam, user) - return self.fake_shell(lambda: mitogen.master.Select.all( + return self.fake_shell(lambda: mitogen.select.Select.all( self._connection.call_async( os.chown, path, ent.pw_uid, ent.pw_gid ) diff --git a/mitogen/core.py b/mitogen/core.py index 80fb589a..6e8856b8 100644 --- a/mitogen/core.py +++ b/mitogen/core.py @@ -33,7 +33,6 @@ import imp import itertools import logging import os -import select import signal import socket import struct @@ -45,6 +44,9 @@ import warnings import weakref import zlib +# Absolute imports for <2.5. +select = __import__('select') + try: import cPickle except ImportError: @@ -531,6 +533,7 @@ class Importer(object): 'lxc', 'master', 'parent', + 'select', 'service', 'setns', 'ssh', diff --git a/mitogen/master.py b/mitogen/master.py index 0805206b..e604fc66 100644 --- a/mitogen/master.py +++ b/mitogen/master.py @@ -181,110 +181,6 @@ class ThreadWatcher(object): return watcher -class SelectError(mitogen.core.Error): - pass - - -class Select(object): - notify = None - - @classmethod - def all(cls, receivers): - return list(msg.unpickle() for msg in cls(receivers)) - - def __init__(self, receivers=(), oneshot=True): - self._receivers = [] - self._oneshot = oneshot - self._latch = mitogen.core.Latch() - for recv in receivers: - self.add(recv) - - def _put(self, value): - self._latch.put(value) - if self.notify: - self.notify(self) - - def __bool__(self): - return bool(self._receivers) - - def __enter__(self): - return self - - def __exit__(self, e_type, e_val, e_tb): - self.close() - - def __iter__(self): - while self._receivers: - yield self.get() - - loop_msg = 'Adding this Select instance would create a Select cycle' - - def _check_no_loop(self, recv): - if recv is self: - raise SelectError(self.loop_msg) - - for recv_ in self._receivers: - if recv_ == recv: - raise SelectError(self.loop_msg) - if isinstance(recv_, Select): - recv_._check_no_loop(recv) - - owned_msg = 'Cannot add: Receiver is already owned by another Select' - - def add(self, recv): - if isinstance(recv, Select): - recv._check_no_loop(self) - - self._receivers.append(recv) - if recv.notify is not None: - raise SelectError(self.owned_msg) - - recv.notify = self._put - # Avoid race by polling once after installation. - if not recv.empty(): - self._put(recv) - - not_present_msg = 'Instance is not a member of this Select' - - def remove(self, recv): - try: - if recv.notify != self._put: - raise ValueError - self._receivers.remove(recv) - recv.notify = None - except (IndexError, ValueError): - raise SelectError(self.not_present_msg) - - def close(self): - for recv in self._receivers[:]: - self.remove(recv) - self._latch.close() - - def empty(self): - return self._latch.empty() - - empty_msg = 'Cannot get(), Select instance is empty' - - def get(self, timeout=None): - if not self._receivers: - raise SelectError(self.empty_msg) - - while True: - recv = self._latch.get(timeout=timeout) - try: - msg = recv.get(block=False) - if self._oneshot: - self.remove(recv) - msg.receiver = recv - return msg - except mitogen.core.TimeoutError: - # A receiver may have been queued with no result if another - # thread drained it before we woke up, or because another - # thread drained it between add() calling recv.empty() and - # self._put(). In this case just sleep again. - continue - - class LogForwarder(object): def __init__(self, router): self._router = router diff --git a/mitogen/parent.py b/mitogen/parent.py index 9d66c736..76a91b24 100644 --- a/mitogen/parent.py +++ b/mitogen/parent.py @@ -32,7 +32,6 @@ import getpass import inspect import logging import os -import select import signal import socket import subprocess @@ -44,6 +43,9 @@ import time import types import zlib +# Absolute imports for <2.5. +select = __import__('select') + try: from cStringIO import StringIO as BytesIO except ImportError: diff --git a/mitogen/select.py b/mitogen/select.py new file mode 100644 index 00000000..d5c1b907 --- /dev/null +++ b/mitogen/select.py @@ -0,0 +1,133 @@ +# Copyright 2017, David Wilson +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are met: +# +# 1. Redistributions of source code must retain the above copyright notice, +# this list of conditions and the following disclaimer. +# +# 2. Redistributions in binary form must reproduce the above copyright notice, +# this list of conditions and the following disclaimer in the documentation +# and/or other materials provided with the distribution. +# +# 3. Neither the name of the copyright holder nor the names of its contributors +# may be used to endorse or promote products derived from this software without +# specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" +# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE +# ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE +# LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR +# CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF +# SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS +# INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN +# CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) +# ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE +# POSSIBILITY OF SUCH DAMAGE. + +import mitogen.core + + +class Error(mitogen.core.Error): + pass + + +class Select(object): + notify = None + + @classmethod + def all(cls, receivers): + return list(msg.unpickle() for msg in cls(receivers)) + + def __init__(self, receivers=(), oneshot=True): + self._receivers = [] + self._oneshot = oneshot + self._latch = mitogen.core.Latch() + for recv in receivers: + self.add(recv) + + def _put(self, value): + self._latch.put(value) + if self.notify: + self.notify(self) + + def __bool__(self): + return bool(self._receivers) + + def __enter__(self): + return self + + def __exit__(self, e_type, e_val, e_tb): + self.close() + + def __iter__(self): + while self._receivers: + yield self.get() + + loop_msg = 'Adding this Select instance would create a Select cycle' + + def _check_no_loop(self, recv): + if recv is self: + raise Error(self.loop_msg) + + for recv_ in self._receivers: + if recv_ == recv: + raise Error(self.loop_msg) + if isinstance(recv_, Select): + recv_._check_no_loop(recv) + + owned_msg = 'Cannot add: Receiver is already owned by another Select' + + def add(self, recv): + if isinstance(recv, Select): + recv._check_no_loop(self) + + self._receivers.append(recv) + if recv.notify is not None: + raise Error(self.owned_msg) + + recv.notify = self._put + # Avoid race by polling once after installation. + if not recv.empty(): + self._put(recv) + + not_present_msg = 'Instance is not a member of this Select' + + def remove(self, recv): + try: + if recv.notify != self._put: + raise ValueError + self._receivers.remove(recv) + recv.notify = None + except (IndexError, ValueError): + raise Error(self.not_present_msg) + + def close(self): + for recv in self._receivers[:]: + self.remove(recv) + self._latch.close() + + def empty(self): + return self._latch.empty() + + empty_msg = 'Cannot get(), Select instance is empty' + + def get(self, timeout=None): + if not self._receivers: + raise Error(self.empty_msg) + + while True: + recv = self._latch.get(timeout=timeout) + try: + msg = recv.get(block=False) + if self._oneshot: + self.remove(recv) + msg.receiver = recv + return msg + except mitogen.core.TimeoutError: + # A receiver may have been queued with no result if another + # thread drained it before we woke up, or because another + # thread drained it between add() calling recv.empty() and + # self._put(). In this case just sleep again. + continue diff --git a/mitogen/service.py b/mitogen/service.py index 0d4bd304..01a6454c 100644 --- a/mitogen/service.py +++ b/mitogen/service.py @@ -31,7 +31,7 @@ import sys import threading import mitogen.core -import mitogen.master +import mitogen.select from mitogen.core import LOG @@ -314,7 +314,7 @@ class Pool(object): self.router = router self.services = list(services) self.size = size - self._select = mitogen.master.Select( + self._select = mitogen.select.Select( receivers=[ service.recv for service in self.services diff --git a/tests/select_test.py b/tests/select_test.py index b057d322..d9345954 100644 --- a/tests/select_test.py +++ b/tests/select_test.py @@ -1,13 +1,13 @@ import unittest2 -import mitogen.master +import mitogen.select import testlib class AddTest(testlib.RouterMixin, testlib.TestCase): - klass = mitogen.master.Select + klass = mitogen.select.Select def test_receiver(self): recv = mitogen.core.Receiver(self.router) @@ -47,7 +47,7 @@ class AddTest(testlib.RouterMixin, testlib.TestCase): def test_subselect_loop_direct(self): select = self.klass() - exc = self.assertRaises(mitogen.master.SelectError, + exc = self.assertRaises(mitogen.select.Error, lambda: select.add(select)) self.assertEquals(str(exc), self.klass.loop_msg) @@ -58,7 +58,7 @@ class AddTest(testlib.RouterMixin, testlib.TestCase): s0.add(s1) s1.add(s2) - exc = self.assertRaises(mitogen.master.SelectError, + exc = self.assertRaises(mitogen.select.Error, lambda: s2.add(s0)) self.assertEquals(str(exc), self.klass.loop_msg) @@ -66,7 +66,7 @@ class AddTest(testlib.RouterMixin, testlib.TestCase): select = self.klass() recv = mitogen.core.Receiver(self.router) select.add(recv) - exc = self.assertRaises(mitogen.master.SelectError, + exc = self.assertRaises(mitogen.select.Error, lambda: select.add(recv)) self.assertEquals(str(exc), self.klass.owned_msg) @@ -74,18 +74,18 @@ class AddTest(testlib.RouterMixin, testlib.TestCase): select = self.klass() select2 = self.klass() select.add(select2) - exc = self.assertRaises(mitogen.master.SelectError, + exc = self.assertRaises(mitogen.select.Error, lambda: select.add(select2)) self.assertEquals(str(exc), self.klass.owned_msg) class RemoveTest(testlib.RouterMixin, testlib.TestCase): - klass = mitogen.master.Select + klass = mitogen.select.Select def test_empty(self): select = self.klass() recv = mitogen.core.Receiver(self.router) - exc = self.assertRaises(mitogen.master.SelectError, + exc = self.assertRaises(mitogen.select.Error, lambda: select.remove(recv)) self.assertEquals(str(exc), self.klass.not_present_msg) @@ -94,7 +94,7 @@ class RemoveTest(testlib.RouterMixin, testlib.TestCase): recv = mitogen.core.Receiver(self.router) recv2 = mitogen.core.Receiver(self.router) select.add(recv2) - exc = self.assertRaises(mitogen.master.SelectError, + exc = self.assertRaises(mitogen.select.Error, lambda: select.remove(recv)) self.assertEquals(str(exc), self.klass.not_present_msg) @@ -108,7 +108,7 @@ class RemoveTest(testlib.RouterMixin, testlib.TestCase): class CloseTest(testlib.RouterMixin, testlib.TestCase): - klass = mitogen.master.Select + klass = mitogen.select.Select def test_empty(self): select = self.klass() @@ -147,7 +147,7 @@ class CloseTest(testlib.RouterMixin, testlib.TestCase): class EmptyTest(testlib.RouterMixin, testlib.TestCase): - klass = mitogen.master.Select + klass = mitogen.select.Select def test_no_receivers(self): select = self.klass() @@ -172,7 +172,7 @@ class EmptyTest(testlib.RouterMixin, testlib.TestCase): class IterTest(testlib.RouterMixin, testlib.TestCase): - klass = mitogen.master.Select + klass = mitogen.select.Select def test_empty(self): select = self.klass() @@ -187,7 +187,7 @@ class IterTest(testlib.RouterMixin, testlib.TestCase): class OneShotTest(testlib.RouterMixin, testlib.TestCase): - klass = mitogen.master.Select + klass = mitogen.select.Select def test_true_removed_after_get(self): recv = mitogen.core.Receiver(self.router) @@ -212,17 +212,17 @@ class OneShotTest(testlib.RouterMixin, testlib.TestCase): class GetTest(testlib.RouterMixin, testlib.TestCase): - klass = mitogen.master.Select + klass = mitogen.select.Select def test_no_receivers(self): select = self.klass() - exc = self.assertRaises(mitogen.master.SelectError, + exc = self.assertRaises(mitogen.select.Error, lambda: select.get()) self.assertEquals(str(exc), self.klass.empty_msg) def test_timeout_no_receivers(self): select = self.klass() - exc = self.assertRaises(mitogen.master.SelectError, + exc = self.assertRaises(mitogen.select.Error, lambda: select.get(timeout=1.0)) self.assertEquals(str(exc), self.klass.empty_msg) From 4bf3d011042cd3e5dc1848175121d39f07b78aee Mon Sep 17 00:00:00 2001 From: David Wilson Date: Fri, 25 May 2018 00:26:21 +0100 Subject: [PATCH 46/59] select: add missing get(block=..) parameter. --- docs/api.rst | 6 ++++-- mitogen/select.py | 4 ++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/docs/api.rst b/docs/api.rst index ecee398d..2580b11c 100644 --- a/docs/api.rst +++ b/docs/api.rst @@ -151,7 +151,7 @@ contexts. Result processing happens concurrently to new results arriving, so :py:meth:`all` should always be faster. - .. py:method:: get (timeout=None) + .. py:method:: get (timeout=None, block=True) Fetch the next available value from any receiver, or raise :py:class:`mitogen.core.TimeoutError` if no value is available within @@ -162,7 +162,9 @@ contexts. :param float timeout: Timeout in seconds. - + :param bool block: + If :py:data:`False`, immediately raise + :py:class:`mitogen.core.TimeoutError` if the select is empty. :return: :py:class:`mitogen.core.Message` :raises mitogen.core.TimeoutError: diff --git a/mitogen/select.py b/mitogen/select.py index d5c1b907..ce4023a9 100644 --- a/mitogen/select.py +++ b/mitogen/select.py @@ -113,12 +113,12 @@ class Select(object): empty_msg = 'Cannot get(), Select instance is empty' - def get(self, timeout=None): + def get(self, timeout=None, block=True): if not self._receivers: raise Error(self.empty_msg) while True: - recv = self._latch.get(timeout=timeout) + recv = self._latch.get(timeout=timeout, block=block) try: msg = recv.get(block=False) if self._oneshot: From 61365236add51ee5892ad03781a3fdb99af73340 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Fri, 25 May 2018 04:47:59 +0100 Subject: [PATCH 47/59] docs/select: fix up more references, fix headings. --- docs/api.rst | 320 ++++++++++++++++++++------------------- docs/getting_started.rst | 2 +- examples/mitop.py | 3 +- mitogen/core.py | 2 +- 4 files changed, 166 insertions(+), 161 deletions(-) diff --git a/docs/api.rst b/docs/api.rst index 2580b11c..43ae8ecb 100644 --- a/docs/api.rst +++ b/docs/api.rst @@ -71,157 +71,6 @@ be sent to any context that will be used to establish additional child contexts. -.. currentmodule:: mitogen.master - -.. class:: Select (receivers=(), oneshot=True) - - Support scatter/gather asynchronous calls and waiting on multiple - receivers, channels, and sub-Selects. Accepts a sequence of - :py:class:`mitogen.core.Receiver` or :py:class:`mitogen.master.Select` - instances and returns the first value posted to any receiver or select. - - If `oneshot` is :data:`True`, then remove each receiver as it yields a - result; since :py:meth:`__iter__` terminates once the final receiver is - removed, this makes it convenient to respond to calls made in parallel: - - .. code-block:: python - - total = 0 - recvs = [c.call_async(long_running_operation) for c in contexts] - - for msg in mitogen.master.Select(recvs): - print 'Got %s from %s' % (msg, msg.receiver) - total += msg.unpickle() - - # Iteration ends when last Receiver yields a result. - print 'Received total %s from %s receivers' % (total, len(recvs)) - - :py:class:`Select` may drive a long-running scheduler: - - .. code-block:: python - - with mitogen.master.Select(oneshot=False) as select: - while running(): - for msg in select: - process_result(msg.receiver.context, msg.unpickle()) - for context, workfunc in get_new_work(): - select.add(context.call_async(workfunc)) - - :py:class:`Select` may be nested: - - .. code-block:: python - - subselects = [ - mitogen.master.Select(get_some_work()), - mitogen.master.Select(get_some_work()), - mitogen.master.Select([ - mitogen.master.Select(get_some_work()), - mitogen.master.Select(get_some_work()) - ]) - ] - - for msg in mitogen.master.Select(selects): - print msg.unpickle() - - .. py:classmethod:: all (it) - - Take an iterable of receivers and retrieve a :py:class:`Message` from - each, returning the result of calling `msg.unpickle()` on each in turn. - Results are returned in the order they arrived. - - This is sugar for handling batch :py:class:`Context.call_async` - invocations: - - .. code-block:: python - - print('Total disk usage: %.02fMiB' % (sum( - mitogen.master.Select.all( - context.call_async(get_disk_usage) - for context in contexts - ) / 1048576.0 - ),)) - - However, unlike in a naive comprehension such as: - - .. code-block:: python - - sum(context.call_async(get_disk_usage).get().unpickle() - for context in contexts) - - Result processing happens concurrently to new results arriving, so - :py:meth:`all` should always be faster. - - .. py:method:: get (timeout=None, block=True) - - Fetch the next available value from any receiver, or raise - :py:class:`mitogen.core.TimeoutError` if no value is available within - `timeout` seconds. - - On success, the message's :py:attr:`receiver - ` attribute is set to the receiver. - - :param float timeout: - Timeout in seconds. - :param bool block: - If :py:data:`False`, immediately raise - :py:class:`mitogen.core.TimeoutError` if the select is empty. - :return: - :py:class:`mitogen.core.Message` - :raises mitogen.core.TimeoutError: - Timeout was reached. - :raises mitogen.core.LatchError: - :py:meth:`close` has been called, and the underlying latch is no - longer valid. - - .. py:method:: __bool__ () - - Return :data:`True` if any receivers are registered with this select. - - .. py:method:: close () - - Remove the select's notifier function from each registered receiver, - mark the associated latch as closed, and cause any thread currently - sleeping in :py:meth:`get` to be woken with - :py:class:`mitogen.core.LatchError`. - - This is necessary to prevent memory leaks in long-running receivers. It - is called automatically when the Python :keyword:`with` statement is - used. - - .. py:method:: empty () - - Return :data:`True` if calling :py:meth:`get` would block. - - As with :py:class:`Queue.Queue`, :data:`True` may be returned even - though a subsequent call to :py:meth:`get` will succeed, since a - message may be posted at any moment between :py:meth:`empty` and - :py:meth:`get`. - - :py:meth:`empty` may return ``False`` even when :py:meth:`get` would - block if another thread has drained a receiver added to this select. - This can be avoided by only consuming each receiver from a single - thread. - - .. py:method:: __iter__ (self) - - Yield the result of :py:meth:`get` until no receivers remain in the - select, either because `oneshot` is :data:`True`, or each receiver was - explicitly removed via :py:meth:`remove`. - - .. py:method:: add (recv) - - Add the :py:class:`mitogen.core.Receiver` or - :py:class:`mitogen.core.Channel` `recv` to the select. - - .. py:method:: remove (recv) - - Remove the :py:class:`mitogen.core.Receiver` or - :py:class:`mitogen.core.Channel` `recv` from the select. Note that if - the receiver has notified prior to :py:meth:`remove`, then it will - still be returned by a subsequent :py:meth:`get`. This may change in a - future version. - - mitogen.fakessh --------------- @@ -327,7 +176,7 @@ Message Class .. attribute:: receiver The :py:class:`mitogen.core.Receiver` over which the message was last - received. Part of the :py:class:`mitogen.master.Select` interface. + received. Part of the :py:class:`mitogen.select.Select` interface. Defaults to :py:data:`None`. .. attribute:: dst_id @@ -392,7 +241,6 @@ Message Class Router Class ============ - .. currentmodule:: mitogen.core .. class:: Router @@ -1061,7 +909,7 @@ Context Class Asynchronous calls may be dispatched in parallel to multiple contexts and consumed as they complete using - :py:class:`mitogen.master.Select`. + :py:class:`mitogen.select.Select`. .. method:: call (fn, \*args, \*\*kwargs) @@ -1077,7 +925,7 @@ Context Class Receiver Class --------------- +============== .. currentmodule:: mitogen.core @@ -1109,7 +957,7 @@ Receiver Class If not ``None``, a reference to a function invoked as `notify(receiver)` when a new message is delivered to this receiver. - Used by :py:class:`mitogen.master.Select` to implement waiting on + Used by :py:class:`mitogen.select.Select` to implement waiting on multiple receivers. .. py:method:: to_sender () @@ -1186,7 +1034,7 @@ Receiver Class Sender Class ------------- +============ .. currentmodule:: mitogen.core @@ -1213,8 +1061,164 @@ Sender Class Send `data` to the remote end. +Select Class +============ + +.. module:: mitogen.select + +.. currentmodule:: mitogen.select + +.. class:: Select (receivers=(), oneshot=True) + + Support scatter/gather asynchronous calls and waiting on multiple + receivers, channels, and sub-Selects. Accepts a sequence of + :py:class:`mitogen.core.Receiver` or :py:class:`mitogen.select.Select` + instances and returns the first value posted to any receiver or select. + + If `oneshot` is :data:`True`, then remove each receiver as it yields a + result; since :py:meth:`__iter__` terminates once the final receiver is + removed, this makes it convenient to respond to calls made in parallel: + + .. code-block:: python + + total = 0 + recvs = [c.call_async(long_running_operation) for c in contexts] + + for msg in mitogen.select.Select(recvs): + print 'Got %s from %s' % (msg, msg.receiver) + total += msg.unpickle() + + # Iteration ends when last Receiver yields a result. + print 'Received total %s from %s receivers' % (total, len(recvs)) + + :py:class:`Select` may drive a long-running scheduler: + + .. code-block:: python + + with mitogen.select.Select(oneshot=False) as select: + while running(): + for msg in select: + process_result(msg.receiver.context, msg.unpickle()) + for context, workfunc in get_new_work(): + select.add(context.call_async(workfunc)) + + :py:class:`Select` may be nested: + + .. code-block:: python + + subselects = [ + mitogen.select.Select(get_some_work()), + mitogen.select.Select(get_some_work()), + mitogen.select.Select([ + mitogen.select.Select(get_some_work()), + mitogen.select.Select(get_some_work()) + ]) + ] + + for msg in mitogen.select.Select(selects): + print msg.unpickle() + + .. py:classmethod:: all (it) + + Take an iterable of receivers and retrieve a :py:class:`Message` from + each, returning the result of calling `msg.unpickle()` on each in turn. + Results are returned in the order they arrived. + + This is sugar for handling batch :py:class:`Context.call_async` + invocations: + + .. code-block:: python + + print('Total disk usage: %.02fMiB' % (sum( + mitogen.select.Select.all( + context.call_async(get_disk_usage) + for context in contexts + ) / 1048576.0 + ),)) + + However, unlike in a naive comprehension such as: + + .. code-block:: python + + sum(context.call_async(get_disk_usage).get().unpickle() + for context in contexts) + + Result processing happens concurrently to new results arriving, so + :py:meth:`all` should always be faster. + + .. py:method:: get (timeout=None, block=True) + + Fetch the next available value from any receiver, or raise + :py:class:`mitogen.core.TimeoutError` if no value is available within + `timeout` seconds. + + On success, the message's :py:attr:`receiver + ` attribute is set to the receiver. + + :param float timeout: + Timeout in seconds. + :param bool block: + If :py:data:`False`, immediately raise + :py:class:`mitogen.core.TimeoutError` if the select is empty. + :return: + :py:class:`mitogen.core.Message` + :raises mitogen.core.TimeoutError: + Timeout was reached. + :raises mitogen.core.LatchError: + :py:meth:`close` has been called, and the underlying latch is no + longer valid. + + .. py:method:: __bool__ () + + Return :data:`True` if any receivers are registered with this select. + + .. py:method:: close () + + Remove the select's notifier function from each registered receiver, + mark the associated latch as closed, and cause any thread currently + sleeping in :py:meth:`get` to be woken with + :py:class:`mitogen.core.LatchError`. + + This is necessary to prevent memory leaks in long-running receivers. It + is called automatically when the Python :keyword:`with` statement is + used. + + .. py:method:: empty () + + Return :data:`True` if calling :py:meth:`get` would block. + + As with :py:class:`Queue.Queue`, :data:`True` may be returned even + though a subsequent call to :py:meth:`get` will succeed, since a + message may be posted at any moment between :py:meth:`empty` and + :py:meth:`get`. + + :py:meth:`empty` may return ``False`` even when :py:meth:`get` would + block if another thread has drained a receiver added to this select. + This can be avoided by only consuming each receiver from a single + thread. + + .. py:method:: __iter__ (self) + + Yield the result of :py:meth:`get` until no receivers remain in the + select, either because `oneshot` is :data:`True`, or each receiver was + explicitly removed via :py:meth:`remove`. + + .. py:method:: add (recv) + + Add the :py:class:`mitogen.core.Receiver` or + :py:class:`mitogen.core.Channel` `recv` to the select. + + .. py:method:: remove (recv) + + Remove the :py:class:`mitogen.core.Receiver` or + :py:class:`mitogen.core.Channel` `recv` from the select. Note that if + the receiver has notified prior to :py:meth:`remove`, then it will + still be returned by a subsequent :py:meth:`get`. This may change in a + future version. + + Channel Class -------------- +============= .. currentmodule:: mitogen.core diff --git a/docs/getting_started.rst b/docs/getting_started.rst index ddaccf87..985f796a 100644 --- a/docs/getting_started.rst +++ b/docs/getting_started.rst @@ -256,7 +256,7 @@ without the need for writing asynchronous code:: contexts = [router.ssh(hostname=hn) for hn in hostnames] calls = [context.call(my_func) for context in contexts] - for recv, (msg, data) in mitogen.master.Select(calls): + for msg in mitogen.select.Select(calls): print 'Reply from %s: %s' % (recv.context, data) diff --git a/examples/mitop.py b/examples/mitop.py index 89de9aad..1cfd92dd 100644 --- a/examples/mitop.py +++ b/examples/mitop.py @@ -18,6 +18,7 @@ import time import mitogen.core import mitogen.master +import mitogen.select import mitogen.utils @@ -185,7 +186,7 @@ def main(router): sys.exit(1) delay = 2.0 - select = mitogen.master.Select(oneshot=False) + select = mitogen.select.Select(oneshot=False) hosts = [] # For each hostname on the command line, create a Host instance, a Mitogen diff --git a/mitogen/core.py b/mitogen/core.py index 6e8856b8..2a5049f2 100644 --- a/mitogen/core.py +++ b/mitogen/core.py @@ -1421,7 +1421,7 @@ class Router(object): return if policy and not policy(msg, stream): - LOG.error('%r: policy refused message: %r', self, msg) + LOG.error('%r: policy for %r refused message: %r', self, fn, msg) if msg.reply_to: self.route(Message.pickled( CallError(self.refused_msg), From d2714752eed28615871efd2205b1494755fc98ef Mon Sep 17 00:00:00 2001 From: David Wilson Date: Fri, 25 May 2018 13:51:45 +0100 Subject: [PATCH 48/59] docs: tidy ups --- docs/api.rst | 128 +++++++++++++------------------------------- docs/howitworks.rst | 37 ++++++------- mitogen/__init__.py | 30 +++-------- mitogen/core.py | 6 +++ mitogen/fakessh.py | 84 +++++++++++++++++++++++++++++ mitogen/master.py | 7 +++ mitogen/parent.py | 6 +++ 7 files changed, 166 insertions(+), 132 deletions(-) diff --git a/docs/api.rst b/docs/api.rst index 43ae8ecb..20389180 100644 --- a/docs/api.rst +++ b/docs/api.rst @@ -28,11 +28,7 @@ mitogen Package mitogen.core ------------ -.. module:: mitogen.core - -This module implements most package functionality, but remains separate from -non-essential code in order to reduce its size, since it is also serves as the -bootstrap implementation sent to every new slave context. +.. automodule:: mitogen.core .. currentmodule:: mitogen.core .. decorator:: takes_econtext @@ -63,102 +59,25 @@ bootstrap implementation sent to every new slave context. mitogen.master -------------- -.. module:: mitogen.master +.. automodule:: mitogen.master + -This module implements functionality required by master processes, such as -starting new contexts via SSH. Its size is also restricted, since it must -be sent to any context that will be used to establish additional child -contexts. +mitogen.parent +-------------- + +.. automodule:: mitogen.parent mitogen.fakessh --------------- -.. module:: mitogen.fakessh - -fakessh is a stream implementation that starts a local subprocess with its -environment modified such that ``PATH`` searches for `ssh` return an mitogen -implementation of the SSH command. When invoked, this tool arranges for the -command line supplied by the calling program to be executed in a context -already established by the master process, reusing the master's (possibly -proxied) connection to that context. - -This allows tools like `rsync` and `scp` to transparently reuse the connections -and tunnels already established by the host program to connect to a target -machine, without wasteful redundant SSH connection setup, 3-way handshakes, or -firewall hopping configurations, and enables these tools to be used in -impossible scenarios, such as over `sudo` with ``requiretty`` enabled. - -The fake `ssh` command source is written to a temporary file on disk, and -consists of a copy of the :py:mod:`mitogen.core` source code (just like any -other child context), with a line appended to cause it to connect back to the -host process over an FD it inherits. As there is no reliance on an existing -filesystem file, it is possible for child contexts to use fakessh. - -As a consequence of connecting back through an inherited FD, only one SSH -invocation is possible, which is fine for tools like `rsync`, however in future -this restriction will be lifted. - -Sequence: - - 1. ``fakessh`` Context and Stream created by parent context. The stream's - buffer has a :py:func:`_fakessh_main` :py:data:`CALL_FUNCTION - ` enqueued. - 2. Target program (`rsync/scp/sftp`) invoked, which internally executes - `ssh` from ``PATH``. - 3. :py:mod:`mitogen.core` bootstrap begins, recovers the stream FD - inherited via the target program, established itself as the fakessh - context. - 4. :py:func:`_fakessh_main` :py:data:`CALL_FUNCTION - ` is read by fakessh context, - - a. sets up :py:class:`IoPump` for stdio, registers - stdin_handle for local context. - b. Enqueues :py:data:`CALL_FUNCTION ` for - :py:func:`_start_slave` invoked in target context, - - i. the program from the `ssh` command line is started - ii. sets up :py:class:`IoPump` for `ssh` command line process's - stdio pipes - iii. returns `(control_handle, stdin_handle)` to - :py:func:`_fakessh_main` - - 5. :py:func:`_fakessh_main` receives control/stdin handles from from - :py:func:`_start_slave`, - - a. registers remote's stdin_handle with local :py:class:`IoPump`. - b. sends `("start", local_stdin_handle)` to remote's control_handle - c. registers local :py:class:`IoPump` with - :py:class:`mitogen.core.Broker`. - d. loops waiting for `local stdout closed && remote stdout closed` - - 6. :py:func:`_start_slave` control channel receives `("start", stdin_handle)`, - - a. registers remote's stdin_handle with local :py:class:`IoPump` - b. registers local :py:class:`IoPump` with - :py:class:`mitogen.core.Broker`. - c. loops waiting for `local stdout closed && remote stdout closed` - +.. image:: images/fakessh.png + :align: right +.. automodule:: mitogen.fakessh .. currentmodule:: mitogen.fakessh -.. function:: run (dest, router, args, daedline=None, econtext=None) - - Run the command specified by the argument vector `args` such that ``PATH`` - searches for SSH by the command will cause its attempt to use SSH to - execute a remote program to be redirected to use mitogen to execute that - program using the context `dest` instead. - - :param mitogen.core.Context dest: - The destination context to execute the SSH command line in. - - :param mitogen.core.Router router: +.. autofunction:: run (dest, router, args, daedline=None, econtext=None) - :param list[str] args: - Command line arguments for local program, e.g. - ``['rsync', '/tmp', 'remote:/tmp']`` - - :returns: - Exit status of the child process. Message Class @@ -168,6 +87,11 @@ Message Class .. class:: Message + Messages are the fundamental unit of communication, comprising the fields + from in the :ref:`stream-protocol` header, an optional reference to the + receiving :class:`mitogen.core.Router` for ingress messages, and helper + methods for deserialization and generating replies. + .. attribute:: router The :py:class:`mitogen.core.Router` responsible for routing the @@ -181,16 +105,36 @@ Message Class .. attribute:: dst_id + Integer target context ID. :py:class:`mitogen.core.Router` delivers + messages locally when their :attr:`dst_id` matches + :data:`mitogen.context_id`, otherwise they are routed up or downstream. + .. attribute:: src_id + Integer source context ID. Used as the target of replies if any are + generated. + .. attribute:: auth_id + The context ID under whose authority the message is acting. See + :py:ref:`source-verification`. + .. attribute:: handle + Integer target handle in the destination context. This is one of the + :py:ref:`standard-handles`, or a dynamically generated handle used to + receive a one-time reply, such as the return value of a function call. + .. attribute:: reply_to + Integer target handle to direct any reply to this message. Used to + receive a one-time reply, such as the return value of a function call. + :data:`IS_DEAD` has a special meaning when it appears in this field. + .. attribute:: data + Message data, which may be raw or pickled. + .. attribute:: is_dead :data:`True` if :attr:`reply_to` is set to the magic value diff --git a/docs/howitworks.rst b/docs/howitworks.rst index dde63c14..0b0ae42d 100644 --- a/docs/howitworks.rst +++ b/docs/howitworks.rst @@ -258,7 +258,7 @@ Stream Protocol .. currentmodule:: mitogen.core Once connected, a basic framing protocol is used to communicate between -parent and child: +parent and child. Integers use big endian in their encoded form. .. list-table:: :header-rows: 1 @@ -342,23 +342,6 @@ Masters listen on the following handles: million parent contexts to be created and destroyed before the associated Router must be recreated. -.. _IS_DEAD: -.. currentmodule:: mitogen.core -.. data:: IS_DEAD - - Special value used to signal disconnection or the inability to route a - message, when it appears in the `reply_to` field. Usually causes - :class:`mitogen.core.ChannelError` to be raised when it is received. - - It indicates the sender did not know how to process the message, or wishes - no further messages to be delivered to it. It is used when: - - * a remote receiver is disconnected or explicitly closed. - * a related message could not be delivered due to no route existing for it. - * a router is being torn down, as a sentinel value to notify - :py:meth:`mitogen.core.Router.add_handler` callbacks to clean up. - - Children listen on the following handles: .. _LOAD_MODULE: @@ -478,6 +461,24 @@ Non-master parents also listen on the following handles: ensuring they are cached and deduplicated at each hop in the chain leading to the target context. +Special values for the `reply_to` field: + +.. _IS_DEAD: +.. currentmodule:: mitogen.core +.. data:: IS_DEAD + + Special value used to signal disconnection or the inability to route a + message, when it appears in the `reply_to` field. Usually causes + :class:`mitogen.core.ChannelError` to be raised when it is received. + + It indicates the sender did not know how to process the message, or wishes + no further messages to be delivered to it. It is used when: + + * a remote receiver is disconnected or explicitly closed. + * a related message could not be delivered due to no route existing for it. + * a router is being torn down, as a sentinel value to notify + :py:meth:`mitogen.core.Router.add_handler` callbacks to clean up. + Additional handles are created to receive the result of every function call triggered by :py:meth:`call_async() `. diff --git a/mitogen/__init__.py b/mitogen/__init__.py index 02be6897..96164d9d 100644 --- a/mitogen/__init__.py +++ b/mitogen/__init__.py @@ -36,33 +36,19 @@ be expected. On the slave, it is built dynamically during startup. __version__ = (0, 0, 2) -#: This is ``False`` in slave contexts. It is used in single-file Python -#: programs to avoid reexecuting the program's :py:func:`main` function in the -#: slave. For example: -#: -#: .. code-block:: python -#: -#: def do_work(): -#: os.system('hostname') -#: -#: def main(broker): -#: context = mitogen.master.connect(broker) -#: context.call(do_work) # Causes slave to import __main__. -#: -#: if __name__ == '__main__' and mitogen.is_master: -#: import mitogen.utils -#: mitogen.utils.run_with_broker(main) -#: +#: This is :data:`False` in slave contexts. Previously it was used to prevent +#: re-execution of :mod:`__main__` in single file programs, however that now +#: happens automatically. is_master = True -#: This is ``0`` in a master, otherwise it is a master-generated ID unique to +#: This is `0` in a master, otherwise it is the master-assigned ID unique to #: the slave context used for message routing. context_id = 0 -#: This is ``None`` in a master, otherwise it is the master-generated ID unique -#: to the slave's parent context. +#: This is :data:`None` in a master, otherwise it is the master-assigned ID +#: unique to the slave's parent context. parent_id = None @@ -76,8 +62,8 @@ def main(log_level='INFO', profiling=False): Convenience decorator primarily useful for writing discardable test scripts. - In the master process, when `func` is defined in the ``__main__`` module, - arranges for `func(router)` to be invoked immediately, with + In the master process, when `func` is defined in the :mod:`__main__` + module, arranges for `func(router)` to be invoked immediately, with :py:class:`mitogen.master.Router` construction and destruction handled just as in :py:func:`mitogen.utils.run_with_router`. In slaves, this function does nothing. diff --git a/mitogen/core.py b/mitogen/core.py index 2a5049f2..4fe2292f 100644 --- a/mitogen/core.py +++ b/mitogen/core.py @@ -26,6 +26,12 @@ # ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE # POSSIBILITY OF SUCH DAMAGE. +""" +This module implements most package functionality, but remains separate from +non-essential code in order to reduce its size, since it is also serves as the +bootstrap implementation sent to every new slave context. +""" + import collections import errno import fcntl diff --git a/mitogen/fakessh.py b/mitogen/fakessh.py index 27cb4acd..d3950b64 100644 --- a/mitogen/fakessh.py +++ b/mitogen/fakessh.py @@ -26,6 +26,70 @@ # ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE # POSSIBILITY OF SUCH DAMAGE. +""" +:mod:`mitogen.fakessh` is a stream implementation that starts a subprocess with +its environment modified such that ``PATH`` searches for `ssh` return a Mitogen +implementation of SSH. When invoked, this implementation arranges for the +command line supplied by the caller to be executed in a remote context, reusing +the parent context's (possibly proxied) connection to that remote context. + +This allows tools like `rsync` and `scp` to transparently reuse the connections +and tunnels already established by the host program to connect to a target +machine, without wasteful redundant SSH connection setup, 3-way handshakes, or +firewall hopping configurations, and enables these tools to be used in +impossible scenarios, such as over `sudo` with ``requiretty`` enabled. + +The fake `ssh` command source is written to a temporary file on disk, and +consists of a copy of the :py:mod:`mitogen.core` source code (just like any +other child context), with a line appended to cause it to connect back to the +host process over an FD it inherits. As there is no reliance on an existing +filesystem file, it is possible for child contexts to use fakessh. + +As a consequence of connecting back through an inherited FD, only one SSH +invocation is possible, which is fine for tools like `rsync`, however in future +this restriction will be lifted. + +Sequence: + + 1. ``fakessh`` Context and Stream created by parent context. The stream's + buffer has a :py:func:`_fakessh_main` :py:data:`CALL_FUNCTION + ` enqueued. + 2. Target program (`rsync/scp/sftp`) invoked, which internally executes + `ssh` from ``PATH``. + 3. :py:mod:`mitogen.core` bootstrap begins, recovers the stream FD + inherited via the target program, established itself as the fakessh + context. + 4. :py:func:`_fakessh_main` :py:data:`CALL_FUNCTION + ` is read by fakessh context, + + a. sets up :py:class:`IoPump` for stdio, registers + stdin_handle for local context. + b. Enqueues :py:data:`CALL_FUNCTION ` for + :py:func:`_start_slave` invoked in target context, + + i. the program from the `ssh` command line is started + ii. sets up :py:class:`IoPump` for `ssh` command line process's + stdio pipes + iii. returns `(control_handle, stdin_handle)` to + :py:func:`_fakessh_main` + + 5. :py:func:`_fakessh_main` receives control/stdin handles from from + :py:func:`_start_slave`, + + a. registers remote's stdin_handle with local :py:class:`IoPump`. + b. sends `("start", local_stdin_handle)` to remote's control_handle + c. registers local :py:class:`IoPump` with + :py:class:`mitogen.core.Broker`. + d. loops waiting for `local stdout closed && remote stdout closed` + + 6. :py:func:`_start_slave` control channel receives `("start", stdin_handle)`, + + a. registers remote's stdin_handle with local :py:class:`IoPump` + b. registers local :py:class:`IoPump` with + :py:class:`mitogen.core.Broker`. + c. loops waiting for `local stdout closed && remote stdout closed` +""" + import getopt import inspect import os @@ -330,6 +394,26 @@ def _get_econtext_config(context, sock2): @mitogen.core.takes_econtext @mitogen.core.takes_router def run(dest, router, args, deadline=None, econtext=None): + """ + Run the command specified by `args` such that ``PATH`` searches for SSH by + the command will cause its attempt to use SSH to execute a remote program + to be redirected to use mitogen to execute that program using the context + `dest` instead. + + :param list args: + Argument vector. + :param mitogen.core.Context dest: + The destination context to execute the SSH command line in. + + :param mitogen.core.Router router: + + :param list[str] args: + Command line arguments for local program, e.g. + ``['rsync', '/tmp', 'remote:/tmp']`` + + :returns: + Exit status of the child process. + """ if econtext is not None: mitogen.parent.upgrade_router(econtext) diff --git a/mitogen/master.py b/mitogen/master.py index e604fc66..9a5b7e83 100644 --- a/mitogen/master.py +++ b/mitogen/master.py @@ -26,6 +26,13 @@ # ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE # POSSIBILITY OF SUCH DAMAGE. +""" +This module implements functionality required by master processes, such as +starting new contexts via SSH. Its size is also restricted, since it must +be sent to any context that will be used to establish additional child +contexts. +""" + import dis import imp import inspect diff --git a/mitogen/parent.py b/mitogen/parent.py index 76a91b24..41505934 100644 --- a/mitogen/parent.py +++ b/mitogen/parent.py @@ -26,6 +26,12 @@ # ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE # POSSIBILITY OF SUCH DAMAGE. +""" +This module defines functionality common to master and parent processes. It is +sent to any child context that is due to become a parent, due to recursive +connection. +""" + import errno import fcntl import getpass From dd48c413323b5acc82155805b68259c3d7941101 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Mon, 28 May 2018 02:33:47 +0100 Subject: [PATCH 49/59] Ignore another annoying flake8 message. --- setup.cfg | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.cfg b/setup.cfg index 92051682..bf012c6b 100644 --- a/setup.cfg +++ b/setup.cfg @@ -6,5 +6,5 @@ omit = mitogen/compat/* [flake8] -ignore = E402,E128,W503 +ignore = E402,E128,W503,E731 exclude = mitogen/compat From 633585524f9b162f007e206491279104996bca56 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Mon, 28 May 2018 02:34:12 +0100 Subject: [PATCH 50/59] tests: don't need separate module for id_allocation_test any more This used to be because everything would explode while importing __main__ under py.test, but that was fixed months ago. --- tests/data/id_allocation.py | 10 ---------- tests/id_allocation_test.py | 14 +++++++++++--- 2 files changed, 11 insertions(+), 13 deletions(-) delete mode 100644 tests/data/id_allocation.py diff --git a/tests/data/id_allocation.py b/tests/data/id_allocation.py deleted file mode 100644 index c9aa64b0..00000000 --- a/tests/data/id_allocation.py +++ /dev/null @@ -1,10 +0,0 @@ - -import mitogen.core -import mitogen.parent - - -@mitogen.core.takes_econtext -def allocate_an_id(econtext): - mitogen.parent.upgrade_router(econtext) - return econtext.router.allocate_id() - diff --git a/tests/id_allocation_test.py b/tests/id_allocation_test.py index 2d2c38f7..1e8d8d1e 100644 --- a/tests/id_allocation_test.py +++ b/tests/id_allocation_test.py @@ -2,7 +2,15 @@ import unittest2 import testlib -import id_allocation + +import mitogen.core +import mitogen.parent + + +@mitogen.core.takes_econtext +def allocate_an_id(econtext): + mitogen.parent.upgrade_router(econtext) + return econtext.router.allocate_id() class SlaveTest(testlib.RouterMixin, testlib.TestCase): @@ -12,11 +20,11 @@ class SlaveTest(testlib.RouterMixin, testlib.TestCase): self.assertEquals(1, context.context_id) # First call from slave allocates a block (2..1001) - id_ = context.call(id_allocation.allocate_an_id) + id_ = context.call(allocate_an_id) self.assertEqual(id_, 2) # Second call from slave allocates from block (3..1001) - id_ = context.call(id_allocation.allocate_an_id) + id_ = context.call(allocate_an_id) self.assertEqual(id_, 3) # Subsequent master allocation does not collide From 2310497d55349acdf3a164c17a9d233260f8cee4 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Mon, 28 May 2018 04:54:16 +0100 Subject: [PATCH 51/59] issue #213: core: have Message.reply() log msg for zero reply_to It's easy to call msg.reply() by accident on a message that never had reply_to set, resulting in a "invalid handle" error message coming from router. Instead log a more accurate message on the stack that actualy caused the problem. --- mitogen/core.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/mitogen/core.py b/mitogen/core.py index 4fe2292f..b9ca31ab 100644 --- a/mitogen/core.py +++ b/mitogen/core.py @@ -380,7 +380,10 @@ class Message(object): msg.dst_id = self.src_id msg.handle = self.reply_to vars(msg).update(kwargs) - (self.router or router).route(msg) + if msg.handle: + (self.router or router).route(msg) + else: + LOG.debug('Message.reply(): discarding due to zero handle: %r', msg) def unpickle(self, throw=True, throw_dead=True): """Deserialize `data` into an object.""" From 40c6c6426fa9317f1446f9fbcb7ea4649fec11f5 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Mon, 28 May 2018 04:56:17 +0100 Subject: [PATCH 52/59] issue #213: core: fix test breakage due to log message change --- mitogen/core.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mitogen/core.py b/mitogen/core.py index b9ca31ab..68d82c79 100644 --- a/mitogen/core.py +++ b/mitogen/core.py @@ -1430,7 +1430,7 @@ class Router(object): return if policy and not policy(msg, stream): - LOG.error('%r: policy for %r refused message: %r', self, fn, msg) + LOG.error('%r: policy refused message: %r', self, msg) if msg.reply_to: self.route(Message.pickled( CallError(self.refused_msg), From 49fb25ee1c054167c74ae085dc11e82f245c0049 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Mon, 28 May 2018 04:57:26 +0100 Subject: [PATCH 53/59] issue #213: core: fix shutdown crash due to member variable rename --- mitogen/core.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/mitogen/core.py b/mitogen/core.py index 68d82c79..8231e97d 100644 --- a/mitogen/core.py +++ b/mitogen/core.py @@ -1610,7 +1610,7 @@ class ExternalContext(object): self.config = config def _on_broker_shutdown(self): - self.channel.close() + self.recv.close() def _on_broker_exit(self): if not self.config['profiling']: @@ -1672,7 +1672,6 @@ class ExternalContext(object): in_fd = self.config.get('in_fd', 100) out_fd = self.config.get('out_fd', 1) - self.recv = Receiver(router=self.router, handle=CALL_FUNCTION, policy=has_parent_authority) From fc59f57ba2541b21bf3a6cd1ccdd93e25752ddb8 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Mon, 28 May 2018 04:58:00 +0100 Subject: [PATCH 54/59] issue #213: core: split out import_module() for use in services.py. --- mitogen/core.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/mitogen/core.py b/mitogen/core.py index 8231e97d..f98fc8d2 100644 --- a/mitogen/core.py +++ b/mitogen/core.py @@ -316,6 +316,13 @@ def enable_profiling(): fp.close() +def import_module(modname): + """ + Import `module` and return the attribute named `attr`. + """ + return __import__(modname, None, None, ['']) + + class Message(object): dst_id = None src_id = None @@ -1784,7 +1791,7 @@ class ExternalContext(object): _v and LOG.debug('_dispatch_calls(%r)', data) modname, klass, func, args, kwargs = data - obj = __import__(modname, {}, {}, ['']) + obj = import_module(modname) if klass: obj = getattr(obj, klass) fn = getattr(obj, func) From e118963b3099c220946bb133f8b961a8b715d136 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Mon, 28 May 2018 05:00:11 +0100 Subject: [PATCH 55/59] issue #254: fork: take care not to rely on FD numbers. --- mitogen/fork.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/mitogen/fork.py b/mitogen/fork.py index 151ace9b..8b29ad0c 100644 --- a/mitogen/fork.py +++ b/mitogen/fork.py @@ -136,6 +136,7 @@ class Stream(mitogen.parent.Stream): # Expected by the ExternalContext.main(). os.dup2(childfp.fileno(), 1) os.dup2(childfp.fileno(), 100) + # Overwritten by ExternalContext.main(); we must replace the # parent-inherited descriptors that were closed by Side._on_fork() to # avoid ExternalContext.main() accidentally allocating new files over @@ -148,7 +149,11 @@ class Stream(mitogen.parent.Stream): if devnull != 2: os.dup2(devnull, 2) os.close(devnull) - childfp.close() + + # If we're unlucky, childfp.fileno() may coincidentally be one of our + # desired FDs. In that case closing it breaks ExternalContext.main(). + if childfp.fileno() not in (0, 1, 100): + childfp.close() config = self.get_econtext_config() config['core_src_fd'] = None @@ -156,8 +161,12 @@ class Stream(mitogen.parent.Stream): config['setup_package'] = False if self.on_start: config['on_start'] = self.on_start + try: mitogen.core.ExternalContext(config).main() + except Exception: + # TODO: report exception somehow. + os._exit(72) finally: # Don't trigger atexit handlers, they were copied from the parent. os._exit(0) From 3f595bbc7e09b18c5e928b1c317d894736baff8e Mon Sep 17 00:00:00 2001 From: David Wilson Date: Mon, 28 May 2018 05:02:25 +0100 Subject: [PATCH 56/59] issue #213: use import_module() in parent.py. This dynamic import crap really needs to be ripped out of parent.py again. Static imports work much better for the module loader too. --- mitogen/parent.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/mitogen/parent.py b/mitogen/parent.py index 41505934..6f1b76f4 100644 --- a/mitogen/parent.py +++ b/mitogen/parent.py @@ -492,9 +492,8 @@ def stream_by_method_name(name): """ if name == 'local': name = 'parent' - Stream = None - exec('from mitogen.%s import Stream' % (name,)) - return Stream + module = mitogen.core.import_module('mitogen.' + name) + return module.Stream @mitogen.core.takes_econtext From a4ddef25a1a585349079e7171bdaa7686272e90a Mon Sep 17 00:00:00 2001 From: David Wilson Date: Mon, 28 May 2018 05:05:43 +0100 Subject: [PATCH 57/59] core: move reader/writer debug prints They stop working with kqueue/epoll poller in the old location. Also comment them out again, should never have been checked in uncommented. --- mitogen/core.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/mitogen/core.py b/mitogen/core.py index f98fc8d2..7f2339a4 100644 --- a/mitogen/core.py +++ b/mitogen/core.py @@ -297,9 +297,11 @@ def enable_debug_logging(): _profile_hook = lambda name, func, *args: func(*args) + def enable_profiling(): global _profile_hook - import cProfile, pstats + import cProfile + import pstats def _profile_hook(name, func, *args): profiler = cProfile.Profile() profiler.enable() @@ -1071,8 +1073,6 @@ class Poller(object): def poll(self, timeout=None): _vv and IOLOG.debug('%r.poll(%r)', self, timeout) - IOLOG.debug('readers = %r', self._rfds) - IOLOG.debug('writers = %r', self._wfds) (rfds, wfds, _), _ = io_op(select.select, self._rfds, self._wfds, @@ -1427,7 +1427,7 @@ class Router(object): refused_msg = 'Refused by policy.' def _invoke(self, msg, stream): - #IOLOG.debug('%r._invoke(%r)', self, msg) + # IOLOG.debug('%r._invoke(%r)', self, msg) try: persist, fn, policy = self._handle_map[msg.handle] except KeyError: @@ -1566,7 +1566,10 @@ class Broker(object): stream.on_disconnect(self) def _loop_once(self, timeout=None): - _vv and IOLOG.debug('%r._loop_once(%r)', self, timeout) + _vv and IOLOG.debug('%r._loop_once(%r, %r)', + self, timeout, self.poller) + #IOLOG.debug('readers =\n%s', pformat(self.poller.readers)) + #IOLOG.debug('writers =\n%s', pformat(self.poller.writers)) for (side, func) in self.poller.poll(timeout): self._call(side.stream, func) From 469bde63c25e0e21612094f9cc3432535dc0d11e Mon Sep 17 00:00:00 2001 From: David Wilson Date: Mon, 28 May 2018 05:07:47 +0100 Subject: [PATCH 58/59] parent: fix log message ordering --- mitogen/parent.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mitogen/parent.py b/mitogen/parent.py index 6f1b76f4..9436591e 100644 --- a/mitogen/parent.py +++ b/mitogen/parent.py @@ -830,7 +830,7 @@ class Stream(mitogen.core.Stream): except OSError: e = sys.exc_info()[1] if e.args[0] == errno.ECHILD: - LOG.warn('%r: waitpid(%r) produced ECHILD', self.pid, self) + LOG.warn('%r: waitpid(%r) produced ECHILD', self, self.pid) return raise From 3b0addcfb0574a8fa6c0ebba30a2a205176f8c31 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Mon, 28 May 2018 05:09:19 +0100 Subject: [PATCH 59/59] service: v2. Closes #213 --- ansible_mitogen/connection.py | 33 +- ansible_mitogen/planner.py | 29 +- ansible_mitogen/services.py | 7 - ansible_mitogen/target.py | 24 +- docs/services.rst | 40 ++- examples/service/server.py | 3 - mitogen/core.py | 57 +++- mitogen/service.py | 313 ++++++++++++------ .../lib/action/mitogen_shutdown_all.py | 8 +- tests/service_test.py | 93 ++++++ 10 files changed, 426 insertions(+), 181 deletions(-) create mode 100644 tests/service_test.py diff --git a/ansible_mitogen/connection.py b/ansible_mitogen/connection.py index 2b031a9f..35643d7d 100644 --- a/ansible_mitogen/connection.py +++ b/ansible_mitogen/connection.py @@ -458,13 +458,10 @@ class Connection(ansible.plugins.connection.ConnectionBase): ) ) - dct = mitogen.service.call( - context=self.parent, - handle=ansible_mitogen.services.ContextService.handle, - method='get', - kwargs=mitogen.utils.cast({ - 'stack': stack, - }) + dct = self.parent.call_service( + service_name='ansible_mitogen.services.ContextService', + method_name='get', + stack=mitogen.utils.cast(list(stack)), ) if dct['msg']: @@ -490,13 +487,10 @@ class Connection(ansible.plugins.connection.ConnectionBase): multiple times. """ if self.context: - mitogen.service.call( - context=self.parent, - handle=ansible_mitogen.services.ContextService.handle, - method='put', - kwargs={ - 'context': self.context - } + self.parent.call_service( + service_name='ansible_mitogen.services.ContextService', + method_name='put', + context=self.context ) self.context = None @@ -618,13 +612,10 @@ class Connection(ansible.plugins.connection.ConnectionBase): return self.put_data(out_path, s, mode=st.st_mode, utimes=(st.st_atime, st.st_mtime)) - mitogen.service.call( - context=self.parent, - handle=ansible_mitogen.services.FileService.handle, - method='register', - kwargs={ - 'path': mitogen.utils.cast(in_path) - } + self.parent.call_service( + service_name='ansible_mitogen.services.FileService', + method_name='register', + path=mitogen.utils.cast(in_path) ) self.call( ansible_mitogen.target.transfer_file, diff --git a/ansible_mitogen/planner.py b/ansible_mitogen/planner.py index e1f37fcd..6605686c 100644 --- a/ansible_mitogen/planner.py +++ b/ansible_mitogen/planner.py @@ -182,13 +182,10 @@ class BinaryPlanner(Planner): def _grant_file_service_access(self, invocation): invocation.connection._connect() - mitogen.service.call( - context=invocation.connection.parent, - handle=ansible_mitogen.services.FileService.handle, - method='register', - kwargs={ - 'path': invocation.module_path - } + invocation.connection.parent.call_service( + service_name='ansible_mitogen.services.FileService', + method_name='register', + path=invocation.module_path, ) def plan(self, invocation, **kwargs): @@ -301,16 +298,14 @@ class NewStylePlanner(ScriptPlanner): def get_module_utils(self, invocation): invocation.connection._connect() - return mitogen.service.call( - context=invocation.connection.parent, - handle=ansible_mitogen.services.ModuleDepService.handle, - method='scan', - kwargs={ - 'module_name': 'ansible_module_%s' % (invocation.module_name,), - 'module_path': invocation.module_path, - 'search_path': self.get_search_path(invocation), - 'builtin_path': module_common._MODULE_UTILS_PATH, - } + return invocation.connection.parent.call_service( + service_name='ansible_mitogen.services.ModuleDepService', + method_name='scan', + + module_name='ansible_module_%s' % (invocation.module_name,), + module_path=invocation.module_path, + search_path=self.get_search_path(invocation), + builtin_path=module_common._MODULE_UTILS_PATH, ) def plan(self, invocation): diff --git a/ansible_mitogen/services.py b/ansible_mitogen/services.py index 0f5be35d..24e1f5b1 100644 --- a/ansible_mitogen/services.py +++ b/ansible_mitogen/services.py @@ -76,8 +76,6 @@ class ContextService(mitogen.service.Service): processes and arranging for the worker to select one according to a hash of the connection parameters (sharding). """ - handle = 500 - max_message_size = 1000 max_interpreters = int(os.getenv('MITOGEN_MAX_INTERPRETERS', '20')) def __init__(self, *args, **kwargs): @@ -420,8 +418,6 @@ class FileService(mitogen.service.Service): proceed normally, without the associated thread needing to be forcefully killed. """ - handle = 501 - max_message_size = 1000 unregistered_msg = 'Path is not registered with FileService.' context_mismatch_msg = 'sender= kwarg context must match requestee context' @@ -609,9 +605,6 @@ class ModuleDepService(mitogen.service.Service): Scan a new-style module and produce a cached mapping of module_utils names to their resolved filesystem paths. """ - max_message_size = 1000 - handle = 502 - def __init__(self, file_service, **kwargs): super(ModuleDepService, self).__init__(**kwargs) self._file_service = file_service diff --git a/ansible_mitogen/target.py b/ansible_mitogen/target.py index 05deacf0..090730d8 100644 --- a/ansible_mitogen/target.py +++ b/ansible_mitogen/target.py @@ -90,26 +90,20 @@ def _get_file(context, path, out_fp): LOG.debug('_get_file(): fetching %r from %r', path, context) t0 = time.time() recv = mitogen.core.Receiver(router=context.router) - metadata = mitogen.service.call( - context=context, - handle=ansible_mitogen.services.FileService.handle, - method='fetch', - kwargs={ - 'path': path, - 'sender': recv.to_sender() - } + metadata = context.call_service( + service_name='ansible_mitogen.services.FileService', + method_name='fetch', + path=path, + sender=recv.to_sender(), ) for chunk in recv: s = chunk.unpickle() LOG.debug('_get_file(%r): received %d bytes', path, len(s)) - mitogen.service.call_async( - context=context, - handle=ansible_mitogen.services.FileService.handle, - method='acknowledge', - kwargs={ - 'size': len(s), - } + context.call_service_async( + service_name='ansible_mitogen.services.FileService', + method_name='acknowledge', + size=len(s), ).close() out_fp.write(s) diff --git a/docs/services.rst b/docs/services.rst index 0c12cee9..4c3f0ab1 100644 --- a/docs/services.rst +++ b/docs/services.rst @@ -6,14 +6,48 @@ Service Framework ================= Mitogen includes a simple framework for implementing services exposed to other -contexts, with built-in subclasses that capture some common service models. -This is a work in progress, and new functionality will be added as common usage -patterns emerge. +contexts, with some built-in subclasses to capture common designs. This is a +work in progress, and new functionality will be added as common usage patterns +emerge. Overview -------- +Service + +* User-supplied class with explicitly exposed methods. +* Identified in calls by its canonical name (e.g. mypkg.mymod.MyClass). +* May be auto-imported/constructed in a child from a parent simply by calling it +* Children receive refusals if the class is not already activated by a aprent +* Has an associated Select instance which may be dynamically loaded with + receivers over time, on_message_received() invoked if any receiver becomes + ready. + +Invoker + +* Abstracts mechanism for calling a service method and verifying permissions. +* Built-in 'service.Invoker': concurrent execution of all methods on the thread pool. +* Built-in 'service.DeduplicatingInvoker': requests are aggregated by distinct + (method, kwargs) key, only one such method executes, return value is cached + and broadcast to all requesters. + +Activator + +* Abstracts mechanism for activating a service and verifying activation + permission. +* Built-in activator looks for service by fully.qualified.ClassName using + Python import mechanism, and only permits parents to trigger activation. + +Pool + +* Manages a fixed-size thread pool, mapping of service name to Invoker, and an + aggregate Select over every activate service's Selects. +* Constructed automatically in children in response to the first + CALL_SERVICE message sent to them by a parent. +* Must be constructed manually in parent context. +* Has close() and add() methods. + Example ------- diff --git a/examples/service/server.py b/examples/service/server.py index 659e677c..2f488d20 100644 --- a/examples/service/server.py +++ b/examples/service/server.py @@ -11,9 +11,6 @@ import mitogen.unix class PingService(mitogen.service.Service): - well_known_id = 500 - max_message_size = 1000 - def dispatch(self, dct, msg): return 'Hello, world' diff --git a/mitogen/core.py b/mitogen/core.py index 7f2339a4..ccaf9ab0 100644 --- a/mitogen/core.py +++ b/mitogen/core.py @@ -75,6 +75,10 @@ IOLOG.setLevel(logging.INFO) _v = False _vv = False +# Also taken by Broker, no blocking work can occur with it held. +_service_call_lock = threading.Lock() +_service_calls = [] + GET_MODULE = 100 CALL_FUNCTION = 101 FORWARD_LOG = 102 @@ -85,6 +89,7 @@ SHUTDOWN = 106 LOAD_MODULE = 107 FORWARD_MODULE = 108 DETACHING = 109 +CALL_SERVICE = 110 IS_DEAD = 999 try: @@ -1000,12 +1005,6 @@ class Context(object): _v and LOG.debug('%r.on_disconnect()', self) fire(self, 'disconnect') - def send(self, msg): - """send `obj` to `handle`, and tell the broker we have output. May - be called from any thread.""" - msg.dst_id = self.context_id - self.router.route(msg) - def send_async(self, msg, persist=False): if self.router.broker._thread == threading.currentThread(): # TODO raise SystemError('Cannot making blocking call on broker thread') @@ -1018,6 +1017,25 @@ class Context(object): self.send(msg) return receiver + def call_service_async(self, service_name, method_name, **kwargs): + _v and LOG.debug('%r.call_service_async(%r, %r, %r)', + self, service_name, method_name, kwargs) + if not isinstance(service_name, basestring): + service_name = service_name.name() # Service.name() + tup = (service_name, method_name, kwargs) + msg = Message.pickled(tup, handle=CALL_SERVICE) + return self.send_async(msg) + + def send(self, msg): + """send `obj` to `handle`, and tell the broker we have output. May + be called from any thread.""" + msg.dst_id = self.context_id + self.router.route(msg) + + def call_service(self, service_name, method_name, **kwargs): + recv = self.call_service_async(service_name, method_name, **kwargs) + return recv.get().unpickle() + def send_await(self, msg, deadline=None): """Send `msg` and wait for a response with an optional timeout.""" receiver = self.send_async(msg) @@ -1626,6 +1644,28 @@ class ExternalContext(object): if not self.config['profiling']: os.kill(os.getpid(), signal.SIGTERM) + def _on_call_service_msg(self, msg): + """ + Stub CALL_SERVICE handler, push message on temporary queue and invoke + _on_stub_call() from the main thread. + """ + if msg.is_dead: + return + _service_call_lock.acquire() + try: + _service_calls.append(msg) + finally: + _service_call_lock.release() + + self.router.route( + Message.pickled( + dst_id=mitogen.context_id, + handle=CALL_FUNCTION, + obj=('mitogen.service', None, '_on_stub_call', (), {}), + router=self.router, + ) + ) + def _on_shutdown_msg(self, msg): _v and LOG.debug('_on_shutdown_msg(%r)', msg) if not msg.is_dead: @@ -1673,6 +1713,11 @@ class ExternalContext(object): handle=SHUTDOWN, policy=has_parent_authority, ) + self.router.add_handler( + fn=self._on_call_service_msg, + handle=CALL_SERVICE, + policy=has_parent_authority, + ) self.master = Context(self.router, 0, 'master') parent_id = self.config['parent_ids'][0] if parent_id == 0: diff --git a/mitogen/service.py b/mitogen/service.py index 01a6454c..6719f833 100644 --- a/mitogen/service.py +++ b/mitogen/service.py @@ -35,23 +35,36 @@ import mitogen.select from mitogen.core import LOG -class Policy(object): - """ - Base security policy. - """ - def is_authorized(self, service, msg): - raise NotImplementedError() +DEFAULT_POOL_SIZE = 16 +_pool = None -class AllowAny(Policy): - def is_authorized(self, service, msg): - return True +@mitogen.core.takes_router +def get_or_create_pool(size=None, router=None): + global _pool + if _pool is None: + _pool = Pool(router, [], size=size or DEFAULT_POOL_SIZE) + return _pool -class AllowParents(Policy): - def is_authorized(self, service, msg): - return (msg.auth_id in mitogen.parent_ids or - msg.auth_id == mitogen.context_id) +@mitogen.core.takes_router +def _on_stub_call(router): + """ + Called for each message received by the core.py stub CALL_SERVICE handler. + Create the pool if it doesn't already exist, and push enqueued messages + into the pool's receiver. This may be called more than once as the stub + service handler runs in asynchronous context, while _on_stub_call() happens + on the main thread. Multiple CALL_SERVICE may end up enqueued before Pool + has a chance to install the real CALL_SERVICE handler. + """ + pool = get_or_create_pool(router=router) + mitogen.core._service_call_lock.acquire() + try: + for msg in mitogen.core._service_calls: + pool._receiver._on_receive(msg) + del mitogen.core._service_calls[:] + finally: + mitogen.core._service_call_lock.release() def validate_arg_spec(spec, args): @@ -132,63 +145,86 @@ def no_reply(): return wrapper -class Service(object): - #: Sentinel object to suppress reply generation, since returning ``None`` - #: will trigger a response message containing the pickled ``None``. - NO_REPLY = object() +class Error(Exception): + """ + Raised when an error occurs configuring a service or pool. + """ - #: If ``None``, a handle is dynamically allocated, otherwise the fixed - #: integer handle to use. - handle = None - max_message_size = 0 - def __init__(self, router): - self.router = router - self.recv = mitogen.core.Receiver(router, self.handle) - self.recv.service = self - self.handle = self.recv.handle - self.running = True +class Policy(object): + """ + Base security policy. + """ + def is_authorized(self, service, msg): + raise NotImplementedError() - def __repr__(self): - return '%s()' % (self.__class__.__name__,) - def on_shutdown(self): - """ - Called by Pool.shutdown() once the last worker thread has exitted. - """ +class AllowAny(Policy): + def is_authorized(self, service, msg): + return True - def dispatch(self, args, msg): - raise NotImplementedError() - def _validate_message(self, msg): - if len(msg.data) > self.max_message_size: - raise mitogen.core.CallError('Message size exceeded.') +class AllowParents(Policy): + def is_authorized(self, service, msg): + return (msg.auth_id in mitogen.parent_ids or + msg.auth_id == mitogen.context_id) - pair = msg.unpickle(throw=False) - if not (isinstance(pair, tuple) and - len(pair) == 2 and - isinstance(pair[0], basestring)): - raise mitogen.core.CallError('Invalid message format.') - method_name, kwargs = pair - method = getattr(self, method_name, None) +class Activator(object): + """ + """ + def is_permitted(self, mod_name, class_name, msg): + return mitogen.core.has_parent_authority(msg) + + not_active_msg = ( + 'Service %r is not yet activated in this context, and the ' + 'caller is not privileged, therefore autoactivation is disabled.' + ) + + def activate(self, pool, service_name, msg): + mod_name, _, class_name = service_name.rpartition('.') + if not self.is_permitted(mod_name, class_name, msg): + raise mitogen.core.CallError(self.not_active_msg, service_name) + + module = mitogen.core.import_module(mod_name) + klass = getattr(module, class_name) + service = klass(pool.router) + pool.add(service) + return service + + +class Invoker(object): + def __init__(self, service): + self.service = service + + def __repr__(self): + return '%s(%s)' % (type(self).__name__, self.service) + + unauthorized_msg = ( + 'Caller is not authorized to invoke %r of service %r' + ) + + def _validate(self, method_name, kwargs, msg): + method = getattr(self.service, method_name, None) if method is None: - raise mitogen.core.CallError('No such method exists.') + raise mitogen.core.CallError('No such method: %r', method_name) policies = getattr(method, 'mitogen_service__policies', None) if not policies: raise mitogen.core.CallError('Method has no policies set.') - if not all(p.is_authorized(self, msg) for p in policies): - raise mitogen.core.CallError('Unauthorized') + if not all(p.is_authorized(self.service, msg) for p in policies): + raise mitogen.core.CallError( + self.unauthorized_msg, + method_name, + self.service.name() + ) required = getattr(method, 'mitogen_service__arg_spec', {}) validate_arg_spec(required, kwargs) - return method_name, kwargs - def _on_receive_message(self, msg): - method_name, kwargs = self._validate_message(msg) - method = getattr(self, method_name) + def _invoke(self, method_name, kwargs, msg): + method = getattr(self.service, method_name) if 'msg' in method.func_code.co_varnames: kwargs['msg'] = msg # TODO: hack @@ -197,7 +233,7 @@ class Service(object): try: ret = method(**kwargs) if no_reply: - return self.NO_REPLY + return Service.NO_REPLY return ret except Exception: if no_reply: @@ -206,22 +242,14 @@ class Service(object): else: raise - def on_receive_message(self, msg): - try: - response = self._on_receive_message(msg) - if response is not self.NO_REPLY: - msg.reply(response) - except mitogen.core.CallError: - e = sys.exc_info()[1] - LOG.warning('%r: call error: %s: %s', self, msg, e) - msg.reply(e) - except Exception: - LOG.exception('While invoking %r.dispatch()', self) - e = sys.exc_info()[1] - msg.reply(mitogen.core.CallError(e)) + def invoke(self, method_name, kwargs, msg): + self._validate(method_name, kwargs, msg) + response = self._invoke(method_name, kwargs, msg) + if response is not Service.NO_REPLY: + msg.reply(response) -class DeduplicatingService(Service): +class DeduplicatingInvoker(Invoker): """ A service that deduplicates and caches expensive responses. Requests are deduplicated according to a customizable key, and the single expensive @@ -233,8 +261,8 @@ class DeduplicatingService(Service): Only one pool thread is blocked during generation of the response, regardless of the number of requestors. """ - def __init__(self, router): - super(DeduplicatingService, self).__init__(router) + def __init__(self, service): + super(DeduplicatingInvoker, self).__init__(service) self._responses = {} self._waiters = {} self._lock = threading.Lock() @@ -261,10 +289,8 @@ class DeduplicatingService(Service): finally: self._lock.release() - def _on_receive_message(self, msg): - method_name, kwargs = self._validate_message(msg) + def _invoke(self, method_name, kwargs, msg): key = self.key_from_request(method_name, kwargs) - self._lock.acquire() try: if key in self._responses: @@ -272,7 +298,7 @@ class DeduplicatingService(Service): if key in self._waiters: self._waiters[key].append(msg) - return self.NO_REPLY + return Service.NO_REPLY self._waiters[key] = [msg] finally: @@ -289,7 +315,37 @@ class DeduplicatingService(Service): e = sys.exc_info()[1] self._produce_response(key, mitogen.core.CallError(e)) - return self.NO_REPLY + return Service.NO_REPLY + + +class Service(object): + #: Sentinel object to suppress reply generation, since returning ``None`` + #: will trigger a response message containing the pickled ``None``. + NO_REPLY = object() + + invoker_class = Invoker + + @classmethod + def name(cls): + return '%s.%s' % (cls.__module__, cls.__name__) + + def __init__(self, router): + self.router = router + self.select = mitogen.select.Select() + + def __repr__(self): + return '%s()' % (self.__class__.__name__,) + + def on_message(self, recv, msg): + """ + Called when a message arrives on any of :attr:`select`'s registered + receivers. + """ + + def on_shutdown(self): + """ + Called by Pool.shutdown() once the last worker thread has exitted. + """ class Pool(object): @@ -299,7 +355,7 @@ class Pool(object): Internally this is implemented by subscribing every :py:class:`Service`'s :py:class:`mitogen.core.Receiver` using a single - :py:class:`mitogen.master.Select`, then arranging for every thread to + :py:class:`mitogen.select.Select`, then arranging for every thread to consume messages delivered to that select. In this way the threads are fairly shared by all available services, and no @@ -308,21 +364,33 @@ class Pool(object): There is no penalty for exposing large numbers of services; the list of exposed services could even be generated dynamically in response to your program's configuration or its input data. + + :param mitogen.core.Router router: + Router to listen for ``CALL_SERVICE`` messages on. + :param list services: + Initial list of services to register. """ + activator_class = Activator + def __init__(self, router, services, size=1): - assert size > 0 self.router = router - self.services = list(services) - self.size = size - self._select = mitogen.select.Select( - receivers=[ - service.recv - for service in self.services - ], - oneshot=False, + self._activator = self.activator_class() + self._receiver = mitogen.core.Receiver( + router=router, + handle=mitogen.core.CALL_SERVICE, ) + + self._select = mitogen.select.Select(oneshot=False) + self._select.add(self._receiver) + #: Serialize service construction. + self._lock = threading.Lock() + self._func_by_recv = {self._receiver: self._on_service_call} + self._invoker_by_name = {} + + for service in services: + self.add(service) self._threads = [] - for x in xrange(size): + for x in range(size): thread = threading.Thread( name='mitogen.service.Pool.%x.worker-%d' % (id(self), x,), target=self._worker_main, @@ -330,6 +398,19 @@ class Pool(object): thread.start() self._threads.append(thread) + @property + def size(self): + return len(self._threads) + + def add(self, service): + name = service.name() + if name in self._invoker_by_name: + raise Error('service named %r already registered' % (name,)) + assert service.select not in self._func_by_recv + invoker = service.invoker_class(service) + self._invoker_by_name[name] = invoker + self._func_by_recv[service.select] = service.on_message + closed = False def stop(self): @@ -337,8 +418,45 @@ class Pool(object): self._select.close() for th in self._threads: th.join() - for service in self.services: - service.on_shutdown() + for invoker in self._invoker_by_name.itervalues(): + invoker.service.on_shutdown() + + def get_invoker(self, name, msg): + self._lock.acquire() + try: + invoker = self._invoker_by_name.get(name) + if not invoker: + service = self._activator.activate(self, name, msg) + invoker = service.invoker_class(service) + self._invoker_by_name[name] = invoker + finally: + self._lock.release() + + return invoker + + def _validate(self, msg): + tup = msg.unpickle(throw=False) + if not (isinstance(tup, tuple) and + len(tup) == 3 and + isinstance(tup[0], basestring) and + isinstance(tup[1], basestring) and + isinstance(tup[2], dict)): + raise mitogen.core.CallError('Invalid message format.') + + def _on_service_call(self, recv, msg): + self._validate(msg) + service_name, method_name, kwargs = msg.unpickle() + try: + invoker = self.get_invoker(service_name, msg) + return invoker.invoke(method_name, kwargs, msg) + except mitogen.core.CallError: + e = sys.exc_info()[1] + LOG.warning('%r: call error: %s: %s', self, msg, e) + msg.reply(e) + except Exception: + LOG.exception('While invoking %r._invoke()', self) + e = sys.exc_info()[1] + msg.reply(mitogen.core.CallError(e)) def _worker_run(self): while not self.closed: @@ -349,11 +467,11 @@ class Pool(object): LOG.info('%r: channel or latch closed, exitting: %s', self, e) return - service = msg.receiver.service + func = self._func_by_recv[msg.receiver] try: - service.on_receive_message(msg) + func(msg.receiver, msg) except Exception: - LOG.exception('While handling %r using %r', msg, service) + LOG.exception('While handling %r using %r', msg, func) def _worker_main(self): try: @@ -367,19 +485,6 @@ class Pool(object): th = threading.currentThread() return 'mitogen.service.Pool(%#x, size=%d, th=%r)' % ( id(self), - self.size, + len(self._threads), th.name, ) - - -def call_async(context, handle, method, kwargs=None): - LOG.debug('service.call_async(%r, %r, %r, %r)', - context, handle, method, kwargs) - pair = (method, kwargs or {}) - msg = mitogen.core.Message.pickled(pair, handle=handle) - return context.send_async(msg) - - -def call(context, handle, method, kwargs): - recv = call_async(context, handle, method, kwargs) - return recv.get().unpickle() diff --git a/tests/ansible/lib/action/mitogen_shutdown_all.py b/tests/ansible/lib/action/mitogen_shutdown_all.py index 9af21e11..6ebdbf5c 100644 --- a/tests/ansible/lib/action/mitogen_shutdown_all.py +++ b/tests/ansible/lib/action/mitogen_shutdown_all.py @@ -25,10 +25,8 @@ class ActionModule(ActionBase): self._connection._connect() return { 'changed': True, - 'result': mitogen.service.call( - context=self._connection.parent, - handle=ansible_mitogen.services.ContextService.handle, - method='shutdown_all', - kwargs={} + 'result': self._connection.parent.call_service( + service_name='ansible_mitogen.services.ContextService', + method_name='shutdown_all', ) } diff --git a/tests/service_test.py b/tests/service_test.py new file mode 100644 index 00000000..8cb18258 --- /dev/null +++ b/tests/service_test.py @@ -0,0 +1,93 @@ +import unittest2 + +import mitogen.core +import mitogen.service +import testlib + + +class MyService(mitogen.service.Service): + def __init__(self, router): + super(MyService, self).__init__(router) + self._counter = 0 + + @mitogen.service.expose(policy=mitogen.service.AllowParents()) + def get_id(self): + self._counter += 1 + return self._counter, id(self) + + @mitogen.service.expose(policy=mitogen.service.AllowParents()) + def privileged_op(self): + return 'privileged!' + + @mitogen.service.expose(policy=mitogen.service.AllowAny()) + def unprivileged_op(self): + return 'unprivileged!' + + + +class MyService2(MyService): + """ + A uniquely named service that lets us test framework activation and class + activation separately. + """ + + +def call_service_in(context, service_name, method_name): + return context.call_service(service_name, method_name) + + +class ActivationTest(testlib.RouterMixin, testlib.TestCase): + def test_parent_can_activate(self): + l1 = self.router.fork() + counter, id_ = l1.call_service(MyService, 'get_id') + self.assertEquals(1, counter) + self.assertTrue(isinstance(id_, int)) + + def test_sibling_cannot_activate_framework(self): + l1 = self.router.fork() + l2 = self.router.fork() + exc = self.assertRaises(mitogen.core.CallError, + lambda: l2.call(call_service_in, l1, MyService2.name(), 'get_id')) + self.assertTrue(mitogen.core.Router.refused_msg in exc.args[0]) + + def test_sibling_cannot_activate_service(self): + l1 = self.router.fork() + l2 = self.router.fork() + l1.call_service(MyService, 'get_id') # force framework activation + exc = self.assertRaises(mitogen.core.CallError, + lambda: l2.call(call_service_in, l1, MyService2.name(), 'get_id')) + msg = mitogen.service.Activator.not_active_msg % (MyService2.name(),) + self.assertTrue(msg in exc.args[0]) + + def test_activates_only_once(self): + l1 = self.router.fork() + counter, id_ = l1.call_service(MyService, 'get_id') + counter2, id_2 = l1.call_service(MyService, 'get_id') + self.assertEquals(1, counter) + self.assertEquals(2, counter2) + self.assertEquals(id_, id_2) + + +class PermissionTest(testlib.RouterMixin, testlib.TestCase): + def test_sibling_unprivileged_ok(self): + l1 = self.router.fork() + l1.call_service(MyService, 'get_id') + l2 = self.router.fork() + self.assertEquals('unprivileged!', + l2.call(call_service_in, l1, MyService.name(), 'unprivileged_op')) + + def test_sibling_privileged_bad(self): + l1 = self.router.fork() + l1.call_service(MyService, 'get_id') + l2 = self.router.fork() + exc = self.assertRaises(mitogen.core.CallError, lambda: + l2.call(call_service_in, l1, MyService.name(), 'privileged_op')) + msg = mitogen.service.Invoker.unauthorized_msg % ( + 'privileged_op', + MyService.name(), + ) + self.assertTrue(msg in exc.args[0]) + + +if __name__ == '__main__': + unittest2.main()