Merge remote-tracking branch 'origin/issue479'

* origin/issue479:
  issue #490: log mitogen.unix server-side accept.
  issue #490: have Side._on_fork() empty _fork_refs
  issue #490: prevent double close() destroying unrelated Connection.
  docs: update Changelog; closes #479.
  issue #479: ModuleFinder special case for __main__ on Py3.x.

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

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 <https://github.com/dw/mitogen/issues/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 <https://github.com/dw/mitogen/commit/16ca111e>`_: handle OpenSSH
   7.5 permission denied prompts when ``~/.ssh/config`` rewrites are present.
 

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):

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

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)
 

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