Hooks: Runtime hook for subprocess block launching standalone cmd

[millerlucas]

Description of the issue

In windowed build using PySide2, subprocess.Popen(["cmd"]) no longer work (cmd open then close immediately). I figured out that the issue come from the subprocess hook (since v4.8, pr #6364). If I comment out this file, cmd start showing again and stay alive.

Context information (for bug reports)

• Output of pyinstaller --version: 5.4.1
• Version of Python: 3.7 / 3.8 / 3.9 / 3.10
• Platform: Windows
• How you installed Python: python.org/downloads
• Did you also try this on another platform? Does it work there? → not relevant on other platform.

A minimal example program which shows the error

A cmd shows up at start, if you comment the hook it stays alive, if you don't the cmd disappear instantly.

import subprocess
import sys

from PySide2 import QtWidgets

class CmdExemple(QtWidgets.QMainWindow):
    def __init__(self):
        super().__init__()
        p = subprocess.Popen(["cmd"])

if __name__ == "__main__":
    app = QtWidgets.QApplication(sys.argv)
    window = CmdExemple()
    window.show()
    exitCode = app.exec_()
    sys.exit(exitCode)

[rokm]

I presume you are building with --noconsole/--windowed, and are relying on the fact that cmd.exe sub-process with invalid stream handles opens a new console window for you? In that case, yes, the runtime hook redirecting the unused pipes to DEVNULL in windowed mode will cause your cmd.exe to exit immediately, because it has no stdin to receive commands from.

Forcing the unused pipes to DEVNULL is required to solve a different class of problems within noconsole frozen applications, so it is going to stay. I think the only way to also accommodate this use case is to exempt attempts to spawn cmd from such processing, by checking if self.args[0] (or rather, its basename without extension) is `cmd˙. @bwoodsend, thoughts?

[bwoodsend]

Does the example actually work if ran via pythonw? I would have thought not.

[rokm]

Does the example actually work if ran via pythonw? I would have thought not.

It does, and it opens a new console window. If ran through regular python, it ends up re-using the existing console.

[bwoodsend]

Hmm, surely that would mean that cmd has something special in it to force a terminal window then? Why is cmd different to any other command line application?

[millerlucas]

It does the same with powershell, don't know why. I've also tried to provide STARTUPINFO with SW_SHOW in wShowWindow but that doesn't change anything.

If you provide this flags creationflags=subprocess.CREATE_NEW_CONSOLE to Popen it stops re-using the existing console and always open a new one.

[rokm]

Hmm, surely that would mean that cmd has something special in it to force a terminal window then? Why is cmd different to any other command line application?

I had some time to dig around this problem, and I think the error with stream handles that the subprocess runtime hook is solving is actually our fault.

Test application

import sys
import os
import signal
import shlex
import subprocess

from PySide2 import QtCore, QtWidgets, QtGui

# Identify application type
if getattr(sys, 'frozen', False):
    if os.path.dirname(__file__) == os.path.dirname(sys.executable):
        APP_TYPE = "onedir"
    else:
        APP_TYPE = "onefile"
    
    if not isinstance(sys.stdout, io.IOBase):
        APP_TYPE += ", windowed"
    else:
        APP_TYPE += ", console"
else:
    APP_TYPE = 'unfrozen'


class MainWindow(QtWidgets.QWidget):
    def __init__(self):
        super().__init__()
        
        self.process = None
        
        self.processStatusTimer = QtCore.QTimer()
        self.processStatusTimer.setTimerType(QtCore.Qt.PreciseTimer)
        self.processStatusTimer.setInterval(100)
        self.processStatusTimer.timeout.connect(self._check_subprocess_status)        
        
        # *** UI ***
        self.setWindowTitle(f"Subprocess test ({APP_TYPE})")
        layout = QtWidgets.QFormLayout(self)
        layout.setFieldGrowthPolicy(QtWidgets.QFormLayout.AllNonFixedFieldsGrow)
        
        # Handles info
        self.labelHandleInfo= QtWidgets.QLabel("<b>Handle info:</b>")
        layout.addRow(self.labelHandleInfo)
        self.textEditHandleInfo= QtWidgets.QTextEdit()
        layout.addRow(self.textEditHandleInfo)
        
        # Command line
        self.lineEditCommandLine = QtWidgets.QLineEdit()
        layout.addRow("Command line: ", self.lineEditCommandLine)
        
        # Redirect streams
        self.checkBoxCloseStdin = QtWidgets.QCheckBox("Close stdin")
        layout.addRow(self.checkBoxCloseStdin)
        self.checkBoxCaptureStdout= QtWidgets.QCheckBox("Capture stdout")
        layout.addRow(self.checkBoxCaptureStdout)
        self.checkBoxCaptureStderr= QtWidgets.QCheckBox("Capture stderr")
        layout.addRow(self.checkBoxCaptureStderr)
        
        # Run
        self.buttonRun = QtWidgets.QPushButton("Run")
        layout.addRow(self.buttonRun)
        self.buttonRun.clicked.connect(self._run_subprocess)
        
        # Output
        self.labelOutput = QtWidgets.QLabel("<b>Output:</b>")
        layout.addRow(self.labelOutput)
        self.textEditOutput = QtWidgets.QTextEdit()
        layout.addRow(self.textEditOutput)
        
        # Populate handle info text box
        self.textEditHandleInfo.setText(self._get_handle_info())

    def _get_handle_info(self):
        text = ''
        
        import traceback
        
        # Use the same _winapi module that subprocess uses
        try:
            import _winapi
        except Exception as e:
            return f"Failed to query handle info: Failed to import _winapi: {e}"
            
        text += "STDIN:\n"
        try:
            handle = _winapi.GetStdHandle(_winapi.STD_INPUT_HANDLE)
            text += f"Handle: {handle!r}"
        except Exception as e:
            tb = traceback.format_exc()
            text += f"Failed to query stdin: {e}\n{tb}"
        text += "\n"
        
        text += "STDOUT:\n"
        try:
            handle = _winapi.GetStdHandle(_winapi.STD_OUTPUT_HANDLE)
            text += f"Handle: {handle!r}"
        except Exception as e:
            tb = traceback.format_exc()
            text += f"Failed to query stdout: {e}\n{tb}"
        text += "\n"
        
        text += "STDERR:\n"
        try:
            handle = _winapi.GetStdHandle(_winapi.STD_ERROR_HANDLE)
            text += f"Handle: {handle!r}"
        except Exception as e:
            tb = traceback.format_exc()
            text += f"Failed to query stderr: {e}\n{tb}"
        text += "\n"
        
        return text
        
    def _run_subprocess(self):
        command = self.lineEditCommandLine.text()
        command = shlex.split(command)
        
        print(f"Running command: {command!r}")
        
        self.textEditOutput.setText("")
        
        kwargs = {}
        if self.checkBoxCloseStdin.isChecked():
            kwargs["stdin"] = subprocess.DEVNULL
        if self.checkBoxCaptureStdout.isChecked():
            kwargs["stdout"] = subprocess.PIPE
        if self.checkBoxCaptureStdout.isChecked():
            kwargs["stderr"] = subprocess.PIPE
        
        try:
            self.process = subprocess.Popen(command, **kwargs)
        except Exception as e:
            import traceback
            tb = traceback.format_exc()
            QtWidgets.QMessageBox.critical(self, "Error", f"Failed to spawn process {command!r}:\n{e}\nTB:\n{tb}")
            return
        
        self.processStatusTimer.start()
    
    def _check_subprocess_status(self):
        running = False
        
        if self.process:
            self.process.poll()
            if self.process.returncode is None:
                # Still running
                running = True
            else:
                print(f"Process finished! Object: {self.process!r}")
                
                self.processStatusTimer.stop()
                
                text = (
                    f"Return code: {self.process.returncode}\n\n"
                    f"Stdout:\n{self.process.stdout.read().decode('utf-8') if self.process.stdout else 'N/A'}\n\n"
                    f"Stderr:\n{self.process.stderr.read().decode('utf-8') if self.process.stderr else 'N/A'}\n\n"
                )
                self.textEditOutput.setText(text)
                
                self.process = None
                
        if running:
            self.buttonRun.setText("Running")
            self.buttonRun.setEnabled(False)
        else:
            self.buttonRun.setText("Run")
            self.buttonRun.setEnabled(True)
   

if __name__ == '__main__':
    app = QtWidgets.QApplication(sys.argv)
    signal.signal(signal.SIGINT, signal.SIG_DFL)

    main = MainWindow()
    main.show()

    sys.exit(app.exec_())

The test program allows spawning a sub-process using the command-line entered in the text field, and allows optionally closing stdin (stdin=subprocess.DEVNULL as opposed to leaving it at default None) and optionally redirecting stdout and/or stderr (stdout=subprocess.PIPE as opposed to leaving it at default None; if redirected, the output is displayed in the bottom text field).

The test application was ran:

• unfrozen with pythonw.exe
• frozen with windowed onedir (with subprocess runtime hook removed)
• frozen with windowed onefile (with subprocess runtime hook removed)

The test command was ffmpeg.exe (and also cbc.exe path\to\problem.lp where cbc.exe comes from PuLP from #6359).

Unfrozen variant always worked, regardless the stream closing/redirection settings.

The onedir variant also always worked, regardless the stream closing/redirection settings.

The onefile variant worked only if no streams were redirected at all, or if all of them were redirected (i.e., stdin was closed and stdout/stderr was redirected). With partial redirection, we get OSError: [WinError 6] The handle is invalid, raised in the subprocess module (and not the subprocess' executable).

Looking at subprocess source, the above behavior can be explained by the following two snippets from subprocess._get_handles. First, if all three passed stream handles are None, the function exits immediately. Otherwise, if not all of them are redirected, the error is raised by the _winapi.GetStdHandle call here, here, or here.

That's why the final iteration of the test application also tries running _winapi.GetStdHandle for all three handles are displays the result. When running unfrozen under pythonw or when running frozen in onedir mode, the function returns None for all handles (the underlying C function returns NULL). But in onefile mode, the calls raise exception (the underlying C function returns INVALID_HANDLE_VALUE and the python wrapper then raises exception).

Once we know that the problem is specific to onefile+windowed combination, it is easy to pin-point the source of the problem - we are spawning the child process with invalid handles. And the problematic part is here:

pyinstaller/bootloader/src/pyi_utils.c

Lines 842 to 844 in 7b39785

	si.hStdInput = (void*)_get_osfhandle(fileno(stdin));
	si.hStdOutput = (void*)_get_osfhandle(fileno(stdout));
	si.hStdError = (void*)_get_osfhandle(fileno(stderr));

In windowed mode, the handles are invalid, so the fileno() + _get_osfhandle() combination ends up returning INVALID_HANDLE_VALUE, which has value -1, which we set as handles for the child process' streams. Instead, we should put this part under #if defined(WINDOWED), and set them to NULL.

Then, we can remove the runtime hook, and interfering with stream handles will also solve this problem with interactive shells that require stdin.

[bwoodsend]

Nice. I can't say that I'll miss that runtime hook.