Honor the timeout argument to Popen.communicate if there is no way to...

Honor the timeout argument to Popen.communicate if there is no way to communicate with the child. Fixes a Python 3.5 test failure; add the Python 3.5 subprocess test suite.

.travis.yml

@@ -12,10 +12,15 @@ env:
     - LINT=false
 install:
     # First install a newer pip so that it can use the wheel cache
-    # (only needed until travis upgrades pip to 7.x)
-    - travis_retry pip install -U pip
+    # (only needed until travis upgrades pip to 7.x; note that the 3.5
+    # environment uses pip 7.1 by default)
+    - travis_retry pip install -U pip wheel
     # Then start installing our deps. Note that use of --build-options / --global-options / --install-options
     # disables the cache.
+    # Bug https://bitbucket.org/pypa/wheel/issues/146/wheel-building-fails-on-cpython-350b3
+    # means that installing cython on cpython 3.5 cannot use the wheel cache, which
+    # means that the LINT=true case is quite slow (2.5 minutes). If this takes very long to fix,
+    # we might want to do some refactoring to move installation into the makefile or a script.
     - travis_retry pip install -U tox cython greenlet pep8 pyflakes
 script:
     - if [[ $TRAVIS_PYTHON_VERSION == '2.7' && $LINT == true ]]; then python setup.py develop && make travis_test_linters; elif [[ $LINT == false && $TRAVIS_PYTHON_VERSION == 'pypy' ]]; then python setup.py develop && make fulltoxtest; elif [[ $LINT == false ]]; then python setup.py develop && make fulltoxtest; fi

changelog.rst

@@ -29,6 +29,13 @@
   by gevent (previously, certain encoding errors could result in
   gevent writing invalid/malformed HTTP responses). Reported by Greg
   Higgins and Carlos Sanchez.
+- ``gevent.subprocess.Popen.communicate`` honors a ``timeout``
+  argument even if there is no way to communicate with the child
+  process (none of stdin, stdout and stderr were set to ``PIPE``).
+  Noticed as part of the Python 3.5 test suite for the new function
+  ``subprocess.run`` but impacts all versions (``timeout`` is an
+  official argument under Python 3 and a gevent extension with
+  slightly different semantics under Python 2).
 
 .. _WSGI specification: https://www.python.org/dev/peps/pep-3333/#the-start-response-callable
 

gevent/subprocess.py

@@ -548,6 +548,20 @@ class Popen(object):
         else:
             stderr = None
 
+        # If we were given stdin=stdout=stderr=None, we have no way to
+        # communicate with the child, and thus no greenlets to wait
+        # on. This is a nonsense case, but it comes up in the test
+        # case for Python 3.5 (test_subprocess.py
+        # RunFuncTestCase.test_timeout). Instead, we go directly to
+        # self.wait
+        if not greenlets and timeout is not None:
+            result = self.wait(timeout=timeout)
+            # Python 3 would have already raised, but Python 2 would not
+            # so we need to do that manually
+            if result is None:
+                from gevent.timeout import Timeout
+                raise Timeout(timeout)
+
         done = joinall(greenlets, timeout=timeout)
         if timeout is not None and len(done) != len(greenlets):
             if PY3:

greentest/3.5/version

+3.5.0