From 3435f24e8dc74db974ee1a33e2911ccaf7886f0c Mon Sep 17 00:00:00 2001 From: David Wilson Date: Sun, 27 Jan 2019 16:17:56 +0000 Subject: [PATCH 1/5] issue #479: ModuleFinder special case for __main__ on Py3.x. --- mitogen/master.py | 41 ++++++++++++++++++++++++++++++++----- tests/module_finder_test.py | 27 ++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 5 deletions(-) diff --git a/mitogen/master.py b/mitogen/master.py index aea28785..2930b42b 100644 --- a/mitogen/master.py +++ b/mitogen/master.py @@ -409,9 +409,37 @@ class ModuleFinder(object): if os.path.exists(path) and self._looks_like_script(path): return path + def _get_main_module_defective_python_3x(self, fullname): + """ + Recent versions of Python 3.x introduced an incomplete notion of + importer specs, and in doing so created permanent asymmetry in the + :mod:`pkgutil` interface handling for the `__main__` module. Therefore + we must handle `__main__` specially. + """ + if fullname != '__main__': + return None + + mod = sys.modules.get(fullname) + if not mod: + return None + + path = getattr(mod, '__file__', None) + if not (os.path.exists(path) and self._looks_like_script(path)): + return None + + fp = open(path, 'rb') + try: + source = fp.read() + finally: + fp.close() + + return path, source, False + def _get_module_via_pkgutil(self, fullname): - """Attempt to fetch source code via pkgutil. In an ideal world, this - would be the only required implementation of get_module().""" + """ + Attempt to fetch source code via pkgutil. In an ideal world, this would + be the only required implementation of get_module(). + """ try: # Pre-'import spec' this returned None, in Python3.6 it raises # ImportError. @@ -543,9 +571,12 @@ class ModuleFinder(object): """ self._found_cache[fullname] = (path, source, is_pkg) - get_module_methods = [_get_module_via_pkgutil, - _get_module_via_sys_modules, - _get_module_via_parent_enumeration] + get_module_methods = [ + _get_main_module_defective_python_3x, + _get_module_via_pkgutil, + _get_module_via_sys_modules, + _get_module_via_parent_enumeration, + ] def get_module_source(self, fullname): """Given the name of a loaded module `fullname`, attempt to find its diff --git a/tests/module_finder_test.py b/tests/module_finder_test.py index b6bf2111..bce3b70d 100644 --- a/tests/module_finder_test.py +++ b/tests/module_finder_test.py @@ -50,6 +50,33 @@ class IsStdlibNameTest(testlib.TestCase): self.assertFalse(self.func('mitogen.fakessh')) +class GetMainModuleDefectivePython3x(testlib.TestCase): + klass = mitogen.master.ModuleFinder + + def call(self, fullname): + return self.klass()._get_main_module_defective_python_3x(fullname) + + def test_builtin(self): + self.assertEquals(None, self.call('sys')) + + def test_not_main(self): + self.assertEquals(None, self.call('mitogen')) + + def test_main(self): + import __main__ + + path, source, is_pkg = self.call('__main__') + self.assertTrue(path is not None) + self.assertTrue(os.path.exists(path)) + self.assertEquals(path, __main__.__file__) + fp = open(path, 'rb') + try: + self.assertEquals(source, fp.read()) + finally: + fp.close() + self.assertFalse(is_pkg) + + class GetModuleViaPkgutilTest(testlib.TestCase): klass = mitogen.master.ModuleFinder From bf676aacfe2787b7c9a0ab1a080be106549899d0 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Sun, 27 Jan 2019 16:20:55 +0000 Subject: [PATCH 2/5] docs: update Changelog; closes #479. --- docs/changelog.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index ca9dfdee..3432a674 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -402,6 +402,10 @@ Core Library Since correct group ownership is not required in most scenarios, when this problem is detected, the PTY is allocated and opened directly by the library. +* `#479 `_: Mitogen could fail to + import :mod:`__main__` on Python 3.4 and newer due to a breaking change in + the :mod:`pkgutil` API. The program's main script is now handled specially. + * `16ca111e `_: handle OpenSSH 7.5 permission denied prompts when ``~/.ssh/config`` rewrites are present. From 38a553d42d85861dd9c9eab5039080a9c6928e1b Mon Sep 17 00:00:00 2001 From: David Wilson Date: Sun, 27 Jan 2019 18:46:03 +0000 Subject: [PATCH 3/5] issue #490: prevent double close() destroying unrelated Connection. --- ansible_mitogen/connection.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/ansible_mitogen/connection.py b/ansible_mitogen/connection.py index cdb83c61..bf1b0747 100644 --- a/ansible_mitogen/connection.py +++ b/ansible_mitogen/connection.py @@ -747,12 +747,16 @@ class Connection(ansible.plugins.connection.ConnectionBase): self.broker = None self.router = None - # #420: Ansible executes "meta" actions in the top-level process, - # meaning "reset_connection" will cause :class:`mitogen.core.Latch` FDs - # to be cached and erroneously shared by children on subsequent - # WorkerProcess forks. To handle that, call on_fork() to ensure any - # shared state is discarded. - mitogen.fork.on_fork() + # #420: Ansible executes "meta" actions in the top-level process, + # meaning "reset_connection" will cause :class:`mitogen.core.Latch` + # FDs to be cached and erroneously shared by children on subsequent + # WorkerProcess forks. To handle that, call on_fork() to ensure any + # shared state is discarded. + # #490: only attempt to clean up when it's known that some + # resources exist to cleanup, otherwise later __del__ double-call + # to close() due to GC at random moment may obliterate an unrelated + # Connection's resources. + mitogen.fork.on_fork() def close(self): """ From 7dae88f0f5c17a16a298aac97df097e4caca977d Mon Sep 17 00:00:00 2001 From: David Wilson Date: Sun, 27 Jan 2019 18:46:03 +0000 Subject: [PATCH 4/5] issue #490: have Side._on_fork() empty _fork_refs This is mostly to avoid ugly debugging that depends on the state of GC. Discard sides from _fork_refs after they have been closed. --- mitogen/core.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/mitogen/core.py b/mitogen/core.py index 7a698f43..5cb7761f 100644 --- a/mitogen/core.py +++ b/mitogen/core.py @@ -1415,7 +1415,9 @@ class Side(object): @classmethod def _on_fork(cls): - for side in list(cls._fork_refs.values()): + while cls._fork_refs: + _, side = cls._fork_refs.popitem() + _vv and IOLOG.debug('Side._on_fork() closing %r', side) side.close() def close(self): From b3700766893cac9367dbdb9165d844945fbf43a7 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Sun, 27 Jan 2019 18:46:03 +0000 Subject: [PATCH 5/5] issue #490: log mitogen.unix server-side accept. --- mitogen/unix.py | 1 + 1 file changed, 1 insertion(+) diff --git a/mitogen/unix.py b/mitogen/unix.py index 44139ae5..7cd5edfe 100644 --- a/mitogen/unix.py +++ b/mitogen/unix.py @@ -117,6 +117,7 @@ class Listener(mitogen.core.BasicStream): self, pid, sys.exc_info()[1]) return + LOG.debug('%r: accepted %r', self, stream) stream.accept(sock.fileno(), sock.fileno()) self._router.register(context, stream)