Wrap os.forkpty for waitpid compatibility in the same way that os.fork is. Fixes #650.

changelog.rst

@@ -43,6 +43,10 @@
   top-level of a module is typically not recommended, but this
   situation can arise when monkey-patching existing scripts. Reported
   in :issue:`651` and :issue:`652` by Mike Kaplinskiy.
+- ``SIGCHLD`` and ``waitpid`` now work for the pids returned by the
+  (monkey-patched) ``os.forkpty`` and ``pty.fork`` functions in the
+  same way they do for the ``os.fork`` function. Reported in
+  :issue:`650` by Erich Heine.
 
 .. _WSGI specification: https://www.python.org/dev/peps/pep-3333/#the-start-response-callable
 

gevent/os.py

@@ -30,7 +30,7 @@ be done with the threadpool.
 Child Processes
 ===============
 
-The functions :func:`fork` and (on POSIX) :func:`waitpid` can be used
+The functions :func:`fork` and (on POSIX) :func:`forkpty` and :func:`waitpid` can be used
 to manage child processes.
 
 .. warning::
@@ -150,17 +150,18 @@ if hasattr(os, 'fork'):
         .. note::
 
             The PID returned by this function may not be waitable with
-            either :func:`os.waitpid` or :func:`waitpid` and it may
-            not generate SIGCHLD signals if libev child watchers are
-            or ever have been in use. For example, the
-            :mod:`gevent.subprocess` module uses libev child watchers
-            (which parts of gevent use libev child watchers is subject
-            to change at any time). Most applications should use
-            :func:`fork_and_watch`, which is monkey-patched as the
-            default replacement for :func:`os.fork` and implements the
-            ``fork`` function of this module by default, unless the
-            environment variable ``GEVENT_NOWAITPID`` is defined
-            before this module is imported.
+            either the original :func:`os.waitpid` or this module's
+            :func:`waitpid` and it may not generate SIGCHLD signals if
+            libev child watchers are or ever have been in use. For
+            example, the :mod:`gevent.subprocess` module uses libev
+            child watchers (which parts of gevent use libev child
+            watchers is subject to change at any time). Most
+            applications should use :func:`fork_and_watch`, which is
+            monkey-patched as the default replacement for
+            :func:`os.fork` and implements the ``fork`` function of
+            this module by default, unless the environment variable
+            ``GEVENT_NOWAITPID`` is defined before this module is
+            imported.
 
         .. versionadded:: 1.1b2
         """
@@ -175,6 +176,33 @@ if hasattr(os, 'fork'):
         """
         return fork_gevent()
 
+    if hasattr(os, 'forkpty'):
+        _raw_forkpty = os.forkpty
+
+        def forkpty_gevent():
+            """
+            Forks the process using :func:`os.forkpty` and prepares the
+            child process to continue using gevent before returning.
+
+            Returns a tuple (pid, master_fd). The `master_fd` is *not* put into
+            non-blocking mode.
+
+            Availability: Some Unix systems.
+
+            .. seealso:: This function has the same limitations as :func:`fork_gevent`.
+
+            .. versionadded:: 1.1b5
+            """
+            pid, master_fd = _raw_forkpty()
+            if not pid:
+                reinit()
+            return pid, master_fd
+
+        forkpty = forkpty_gevent
+
+        __implements__.append('forkpty')
+        __extensions__.append("forkpty_gevent")
+
     if hasattr(os, 'WNOWAIT') or hasattr(os, 'WNOHANG'):
         # We can only do this on POSIX
         import time
@@ -317,8 +345,30 @@ if hasattr(os, 'fork'):
         __extensions__.append('fork_and_watch')
         __extensions__.append('fork_gevent')
 
+        if 'forkpty' in __implements__:
+            def forkpty_and_watch(callback=None, loop=None, ref=False, forkpty=forkpty_gevent):
+                """
+                Like :func:`fork_and_watch`, except using :func:`forkpty_gevent`.
+
+                Availability: Some Unix systems.
+
+                .. versionadded:: 1.1b5
+                """
+                result = []
+
+                def _fork():
+                    pid_and_fd = forkpty()
+                    result.append(pid_and_fd)
+                    return pid_and_fd[0]
+                fork_and_watch(callback, loop, ref, _fork)
+                return result[0]
+
+            __extensions__.append('forkpty_and_watch')
+
         # Watch children by default
         if not os.getenv('GEVENT_NOWAITPID'):
+            # Broken out into separate functions instead of simple name aliases
+            # for documentation purposes.
             def fork(*args, **kwargs):
                 """
                 Forks a child process and starts a child watcher for it in the
@@ -332,6 +382,20 @@ if hasattr(os, 'fork'):
                 """
                 # take any args to match fork_and_watch
                 return fork_and_watch(*args, **kwargs)
+
+            if 'forkpty' in __implements__:
+                def forkpty(*args, **kwargs):
+                    """
+                    Like :func:`fork`, but using :func:`forkpty_gevent`.
+
+                    This implementation of ``forkpty`` is a wrapper for :func:`forkpty_and_watch`
+                    when the environment variable ``GEVENT_NOWAITPID`` is *not* defined.
+                    This is the default and should be used by most applications.
+
+                    .. versionadded:: 1.1b5
+                    """
+                    # take any args to match fork_and_watch
+                    return forkpty_and_watch(*args, **kwargs)
             __implements__.append("waitpid")
         else:
             def fork():
@@ -344,6 +408,19 @@ if hasattr(os, 'fork'):
                 This is not recommended for most applications.
                 """
                 return fork_gevent()
+
+            if 'forkpty' in __implements__:
+                def forkpty():
+                    """
+                    Like :func:`fork`, but using :func:`os.forkpty`
+
+                    This implementation of ``forkptf`` is a wrapper for :func:`forkpty_gevent`
+                    when the environment variable ``GEVENT_NOWAITPID`` *is* defined.
+                    This is not recommended for most applications.
+
+                    .. versionadded:: 1.1b5
+                    """
+                    return forkpty_gevent()
             __extensions__.append("waitpid")
 
 else:

greentest/test__monkey.py

@@ -20,8 +20,13 @@ from gevent import socket as gevent_socket
 assert socket.create_connection is gevent_socket.create_connection
 
 import os
-if hasattr(os, 'fork'):
-    assert 'built-in' not in repr(os.fork), repr(os.fork)
+import types
+for name in ('fork', 'forkpty'):
+    if hasattr(os, name):
+        attr = getattr(os, name)
+        assert 'built-in' not in repr(attr), repr(attr)
+        assert not isinstance(attr, types.BuiltinFunctionType), repr(attr)
+        assert isinstance(attr, types.FunctionType), repr(attr)
 
 assert monkey.saved
 

greentest/test__monkey_sigchld.py

@@ -8,7 +8,7 @@ import gevent.monkey
 gevent.monkey.patch_all()
 
 pid = None
-awaiting_child = [True]
+awaiting_child = []
 
 
 def handle_sigchld(*args):
@@ -26,24 +26,44 @@ if hasattr(signal, 'SIGCHLD'):
     handler = signal.getsignal(signal.SIGCHLD)
     assert signal.getsignal(signal.SIGCHLD) is handle_sigchld, handler
 
-    pid = os.fork()
-    if not pid:
-        # child
-        gevent.sleep(0.2)
-        sys.exit(0)
+    if hasattr(os, 'forkpty'):
+        def forkpty():
+            # For printing in errors
+            return os.forkpty()[0]
+        funcs = (os.fork, forkpty)
     else:
-        with gevent.Timeout(1):
-            while awaiting_child:
-                gevent.sleep(0.01)
-            # We should now be able to waitpid() for an arbitrary child
-            wpid, status = os.waitpid(-1, os.WNOHANG)
-            assert wpid == pid
-            # And a second call should raise ECHILD
+        funcs = (os.fork,)
+
+    for func in funcs:
+        awaiting_child = [True]
+        pid = func()
+        if not pid:
+            # child
+            gevent.sleep(0.3)
+            sys.exit(0)
+        else:
+            timeout = gevent.Timeout(1)
             try:
+                while awaiting_child:
+                    gevent.sleep(0.01)
+                # We should now be able to waitpid() for an arbitrary child
                 wpid, status = os.waitpid(-1, os.WNOHANG)
-                raise AssertionError("Should not be able to wait again")
-            except OSError as e:
-                assert e.errno == errno.ECHILD
-            sys.exit(0)
+                if wpid != pid:
+                    raise AssertionError("Failed to wait on a child pid forked with a function",
+                                         wpid, pid, func)
+
+                # And a second call should raise ECHILD
+                try:
+                    wpid, status = os.waitpid(-1, os.WNOHANG)
+                    raise AssertionError("Should not be able to wait again")
+                except OSError as e:
+                    assert e.errno == errno.ECHILD
+            except gevent.Timeout as t:
+                if timeout is not t:
+                    raise
+                raise AssertionError("Failed to wait using", func)
+            finally:
+                timeout.cancel()
+    sys.exit(0)
 else:
     print("No SIGCHLD, not testing")