Hooks: Force all unused subprocess pipes to DEVNULL in windowed mode. (…

…#6364)

A windowed app's stdin, stdout and stderr are not valid pipes. Invoking
subprocess.run() (or any of its siblings) without explicitly setting all three
pipes to either subprocess.PIPE or subprocess.DEVNULL causes pipe inheritance
where subprocess will attempt to use the invalid parent process's pipe. This
causes an error.

Add a runtime hook which monkey-patches subprocess.Popen() so that any unused
pipes are replaced with DEVNULL.

PyInstaller/hooks/rthooks.dat

@@ -23,4 +23,5 @@
     'win32api':   ['pyi_rth_win32api.py'],
     'win32com':   ['pyi_rth_win32comgenpy.py'],
     'multiprocessing': ['pyi_rth_multiprocessing.py'],
+    'subprocess': ['pyi_rth_subprocess.py'],
 }

PyInstaller/hooks/rthooks/pyi_rth_subprocess.py

@@ -0,0 +1,28 @@
+#-----------------------------------------------------------------------------
+# Copyright (c) 2021, PyInstaller Development Team.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+#
+# The full license is in the file COPYING.txt, distributed with this software.
+#
+# SPDX-License-Identifier: Apache-2.0
+#-----------------------------------------------------------------------------
+
+import subprocess
+import sys
+import io
+
+
+class Popen(subprocess.Popen):
+
+    # In windowed mode, force any unused pipes (stdin, stdout and stderr) to be DEVNULL instead of inheriting the
+    # invalid corresponding handles from this parent process.
+    if not isinstance(sys.stdout, io.IOBase):
+
+        def _get_handles(self, stdin, stdout, stderr):
+            stdin, stdout, stderr = (subprocess.DEVNULL if pipe is None else pipe for pipe in (stdin, stdout, stderr))
+            return super()._get_handles(stdin, stdout, stderr)
+
+
+subprocess.Popen = Popen

PyInstaller/utils/conftest.py

@@ -350,12 +350,12 @@ def _run_executable(self, prog, args, run_from_path, runtime):
         # Using sys.stdout/sys.stderr for subprocess fixes printing messages in Windows command prompt. Py.test is then
         # able to collect stdout/sterr messages and display them if a test fails.
         for _ in range(_MAX_RETRIES):
-            retcode = self.__run_executable(args, exe_path, prog_env, prog_cwd, runtime)
+            retcode = self._run_executable_(args, exe_path, prog_env, prog_cwd, runtime)
             if retcode != 1:  # retcode == 1 means a timeout
                 break
         return retcode
 
-    def __run_executable(self, args, exe_path, prog_env, prog_cwd, runtime):
+    def _run_executable_(self, args, exe_path, prog_env, prog_cwd, runtime):
         process = psutil.Popen(
             args, executable=exe_path, stdout=subprocess.PIPE, stderr=subprocess.PIPE, env=prog_env, cwd=prog_cwd
         )
@@ -554,3 +554,18 @@ def compiled_dylib(tmpdir, request):
         old_wd.chdir()
 
     return tmp_data_dir
+
+
+@pytest.fixture
+def pyi_windowed_builder(pyi_builder: AppBuilder):
+    """A pyi_builder equivalent for testing --windowed applications."""
+
+    # psutil.Popen() somehow bypasses an application's windowed/console mode so that any application built in
+    # --windowed mode but invoked with psutil still recieves valid std{in,out,err} handles and behaves exactly like
+    # a console application. In short, testing windowed mode with psutil is a null test. We must instead use subprocess.
+
+    def _run_executable_(args, exe_path, prog_env, prog_cwd, runtime):
+        return subprocess.run([exe_path, *args], env=prog_env, cwd=prog_cwd, timeout=runtime).returncode
+
+    pyi_builder._run_executable_ = _run_executable_
+    yield pyi_builder

news/6364.bugfix.1.rst

@@ -0,0 +1,3 @@
+Prevent invalid handle errors when an application compiled in :option:`--windowed` mode uses :mod:`subprocess` without
+explicitly setting **stdin**, **stdout** and **stderr** to either :data:`~subprocess.PIPE` or
+:data:`~subprocess.DEVNULL`.

tests/functional/test_basic.py

@@ -770,3 +770,32 @@ def MyEXE(*args, **kwargs):
         assert exe_basename == '{}', "Unexpected basename(sys.executable): " + exe_basename
         """.format(exe_basename)
     )
+
+
+@pytest.mark.darwin
+@pytest.mark.win32
+def test_subprocess_in_windowed_mode(pyi_windowed_builder):
+    """Test invoking subprocesses from a PyInstaller app built in windowed mode."""
+
+    pyi_windowed_builder.test_source(
+        r"""
+        from subprocess import PIPE, run
+        from unittest import TestCase
+
+        # Lazily use unittest's rich assertEqual() for assertions with builtin diagnostics.
+        assert_equal = TestCase().assertEqual
+
+        run([{0}, "-c", ""], check=True)
+
+        # Verify that stdin, stdout and stderr still work and haven't been muddled.
+        p = run([{0}, "-c", "print('foo')"], stdout=PIPE, universal_newlines=True)
+        assert_equal(p.stdout, "foo\n", p.stdout)
+
+        p = run([{0}, "-c", r"import sys; sys.stderr.write('bar\n')"], stderr=PIPE, universal_newlines=True)
+        assert_equal(p.stderr, "bar\n", p.stderr)
+
+        p = run([{0}], input="print('foo')\nprint('bar')\n", stdout=PIPE, universal_newlines=True)
+        assert_equal(p.stdout, "foo\nbar\n", p.stdout)
+        """.format(repr(sys.executable)),
+        pyi_args=["--windowed"]
+    )