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): """ 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. 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): 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/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) 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